Bug 63621 - REGRESSION: DOMNodeRemoved doesn't fire when removing text node in contenteditable
Summary: REGRESSION: DOMNodeRemoved doesn't fire when removing text node in contentedi...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.6
: P1 Normal
Assignee: Nobody
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2011-06-29 05:28 PDT by Erik Hesselink
Modified: 2012-06-03 00:22 PDT (History)
5 users (show)

See Also:


Attachments
Testcase showing the bug (1.04 KB, text/html)
2011-06-29 05:28 PDT, Erik Hesselink
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Erik Hesselink 2011-06-29 05:28:39 PDT
Created attachment 99076 [details]
Testcase showing the bug

In the attached test case:

  * Select the text 'foo' by double clicking.
  * Press delete.

In the console you can now see that the text node containing 'foo' has been removed, but no DOMNodeRemoved event has been fired (only DOMCharacterDataModified).
Comment 1 Alexey Proskuryakov 2011-06-29 10:23:48 PDT
I can reproduce with ToT, but not with Safari/WebKit 5.0.5.
Comment 2 Ryosuke Niwa 2011-06-29 10:31:11 PDT
This is a behavioral change since http://trac.webkit.org/changeset/73690, and it's intentional.  The DOMNodeRemoved is fired for the node AFTER the node is removed from the document.  However, because the node does not have a parent after the removal, it appears as if the event was never fired if you just attach an event handler on some ancestor.
Comment 3 Erik Hesselink 2011-06-30 03:06:12 PDT
(In reply to comment #2)
> This is a behavioral change since http://trac.webkit.org/changeset/73690, and it's intentional.  The DOMNodeRemoved is fired for the node AFTER the node is removed from the document.  However, because the node does not have a parent after the removal, it appears as if the event was never fired if you just attach an event handler on some ancestor.

I don't really understand what that changeset does exactly (an explanation would be appreciated), but the current behavior is undesirable for several reasons:

  * It violates the specification. Even though it is deprecated, this is the specification that has been implemented, and such small deviations are confusing and a recipe for bugs.
  * It is inconsistent. Most DOMNodeRemoved events still fire before removal.
  * It is hard to use. Monitoring a section of HTML (say, a contenteditable area) can now only be done by attaching event listeners to every node, which is a kind of web development antipattern.
Comment 4 Ryosuke Niwa 2011-06-30 08:05:13 PDT
(In reply to comment #3)
>   * It violates the specification. Even though it is deprecated, this is the specification that has been implemented, and such small deviations are confusing and a recipe for bugs.

This is a willful violation of the spec.  We've had many crashes and security vulnerabilities due to mutation events.

>   * It is inconsistent. Most DOMNodeRemoved events still fire before removal.

Right.  We'll eventually do that to doo DOMNodeRemoved.  For example, adoptNode does the same.

>   * It is hard to use. Monitoring a section of HTML (say, a contenteditable area) can now only be done by attaching event listeners to every node, which is a kind of web development antipattern.

On WebKit, you can use input event to monitor a contenteditable area. It's fired every time DOM is about to modify in the element.

Ultimately, we'd like to get rid of mutation events. See "Mutation events replacement" on public-webapps.
Comment 5 Erik Hesselink 2011-07-01 06:57:06 PDT
(In reply to comment #4)
> (In reply to comment #3)
> >   * It is hard to use. Monitoring a section of HTML (say, a contenteditable area) can now only be done by attaching event listeners to every node, which is a kind of web development antipattern.
> 
> On WebKit, you can use input event to monitor a contenteditable area. It's fired every time DOM is about to modify in the element.
> 
> Ultimately, we'd like to get rid of mutation events. See "Mutation events replacement" on public-webapps.

I'll give a little bit of background about what we are using DOM mutation events for. We are developing a web application which includes an html editor based on contenteditable. In this contenteditable we have user actions like typing, calls to execCommand and custom actions that modify the DOM directly.

We use DOM mutation events to basically keep a shadow DOM, which we use to have 'live' computations on the DOM that automatically update on DOM changes in a performant way (i.e. without rescanning the entire document). This keeps our UI state correct, among other things.

We also have an automatic undo stack based on DOM mutation events. Each action on the DOM is wrapped in markers, and between these markers we track the events. We can then undo the individual events to undo the full action.

For both these applications (we have a few more, but these are the largest) we need the information and guarantees DOM mutation events give us. The 'input' event doesn't give nearly enough information. The new spec probably also doesn't (I'll send an email to public-webapps soon), and also isn't available yet.

My worry is that these changes, which make it easier for you to fix bugs, keep breaking our application in unexpected ways. Do you have any high level overview of what you are planning to change, and when? This would really help us in ensuring our application will keep working on Safari and Chrome.
Comment 6 Ryosuke Niwa 2011-07-01 08:56:53 PDT
(In reply to comment #5)
> We use DOM mutation events to basically keep a shadow DOM, which we use to have 'live' computations on the DOM that automatically update on DOM changes in a performant way (i.e. without rescanning the entire document). This keeps our UI state correct, among other things.
> 
> We also have an automatic undo stack based on DOM mutation events. Each action on the DOM is wrapped in markers, and between these markers we track the events. We can then undo the individual events to undo the full action.

One thing you can do is to attach DOMNodeRemoved event listener inside DONNodeInserted for each node.  That way, you'll know when a node is removed.

> For both these applications (we have a few more, but these are the largest) we need the information and guarantees DOM mutation events give us. The 'input' event doesn't give nearly enough information. The new spec probably also doesn't (I'll send an email to public-webapps soon), and also isn't available yet.

What I'd like to do in the replacement for DOM mutation events is to send a list of DOM mutations such that you can construct undo and redo.  In fact, I've been working on a brand new design/spec for UndoMananger and Transaction that help address your use case that I'm planning to send out next week or so.

> My worry is that these changes, which make it easier for you to fix bugs, keep breaking our application in unexpected ways. Do you have any high level overview of what you are planning to change, and when? This would really help us in ensuring our application will keep working on Safari and Chrome.

See discussions on https://bugs.webkit.org/show_bug.cgi?id=55666
Comment 7 Ryosuke Niwa 2012-06-03 00:22:41 PDT
Given that we've added mutation observers API, I don't think we'll ever fix this bug.