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

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years 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

2 years 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

2 years 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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ec3adc50935d
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.