elt.firstChild.remove() doesn't trigger a DOMSubtreeModified event

RESOLVED FIXED in Firefox 55

Status

()

Core
DOM: Events
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: florian, Assigned: smaug)

Tracking

54 Branch
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
Replacing
  subtree.firstChild.removeChild(subtree.firstChild.firstChild);
with
  subtree.firstChild.firstChild.remove();

at http://searchfox.org/mozilla-central/rev/d4adaabd6d2f88be0d0b151e022c06f6554909da/dom/events/test/test_bug288392.html#61 makes that test fail, as seen in the try push from bug 1345253 comment 2.
As DOMSubtreeModified is deprecated in DOM level 3, I guess we might not consider to fix this?
Maybe not, but mutation events working inconsistently hint that there might be some issue with remove() implementation.
Yes, nsINode::Remove() is wrong. It calls parent->RemoveChildAt(uint32_t(index), true);
when it most probably should call 
nsINode::RemoveChild(oldChild, errorResult)
Assignee: nobody → bugs
Blocks: 856629
Created attachment 8844875 [details] [diff] [review]
patch

This makes remove() work consistently with removeChild(), including the a bit weird NodeRemoved dispatching when trying to remove anonymous nodes.
But anonymous nodes aren't exposed to the web so this should be fine - at least consistent.
Attachment #8844875 - Flags: review?(Ms2ger)
Comment on attachment 8844875 [details] [diff] [review]
patch

Review of attachment 8844875 [details] [diff] [review]:
-----------------------------------------------------------------

This seems fine to me.
Attachment #8844875 - Flags: review?(Ms2ger) → review+

Comment 6

8 months ago
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec3adc50935d
elt.firstChild.remove() doesn't trigger a DOMSubtreeModified event, r=ms2ger

Comment 7

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ec3adc50935d
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

6 months ago
Blocks: 1335150
You need to log in before you can comment on or make changes to this bug.