search

sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.77k stars 4.14k forks source link

contenteditable with removed by user html nodes inside "if" block throws an error on update #5274

Open boatincow opened 4 years ago

boatincow commented 4 years ago

Describe the bug If you have html nodes inside contenteditable block, which are wrapped with {#if}{/if} — removing that nodes will throw an error after update.

Logs Uncaught (in promise) TypeError: Cannot read property 'removeChild' of null at detach at Array.forEach at HtmlTag.d at HtmlTag.p at Object.update [as p] at Object.update [as p] at update at flush

To Reproduce That behavior is reproducible, here is REPL https://svelte.dev/repl/fa601199d8d345d3977bf48bdb8d14f4?version=3.24.1

ehrencrona commented 4 years ago

This is a duplicate of #4978

boatincow commented 4 years ago

Thanks for pointing that out. May be it would be nice to add remarks in https://svelte.dev/docs about that, or show warning, when there is dynamic code inside contenteditable. At this time tutorial (https://svelte.dev/tutorial/contenteditable-bindings) and docs look like there will be no problems with using contenteditable in any way.

nicolodavis commented 3 years ago

Related: https://github.com/sveltejs/svelte/issues/5326

I also second the idea of adding a warning in the docs. It sounds like having nested tags inside a contenteditable is not supported by Svelte, but a user reading the docs seeing that bind:innerHTML is supported would never guess that.

Quite separately, it's actually quite easy to support this use case with a bit of defensive code (see https://github.com/sveltejs/svelte/issues/5326). I understand that there might be concerns about shipping this code to users that won't need it, but perhaps it's possible to do something clever when a contenteditable is involved specifically?

Someone with knowledge of Svelte internals might be able to shed more light on this.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.