"Assertion failure: insertion.mContainer == GetInsertionPoint(parent, child).mContainer" with createShadowRoot, <xbl:children>

RESOLVED FIXED in Firefox 61

Status

()

defect
P2
critical
RESOLVED FIXED
3 years ago
4 months ago

People

(Reporter: jruderman, Assigned: emilio)

Tracking

(Blocks 1 bug, {assertion, testcase})

Trunk
mozilla61
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox56 wontfix, firefox57 wontfix, firefox58 wontfix, firefox59 wontfix, firefox60 wontfix, firefox61 fixed)

Details

Attachments

(3 attachments)

Posted file testcase
Assertion failure: insertion.mContainer == GetInsertionPoint(parent, child).mContainer, at layout/base/nsCSSFrameConstructor.cpp:10671
Posted file stack
I thought XBL was disabled for web content, so it's disturbing that a testcase with <xbl:children> can make special things happen. Do I need to re-teach my fuzzer to create XBL elements? Which ones?
XBL binding attachment is disabled for web content, with some exceptions.

ChildIterator.cpp has explicit checks for xbl:children and assumes stuff if it sees them.  That's probably broken.  :(  They should probably be using IsActiveChildrenElement() instead or something.  That was added in bug 890775 but apparently not all consumers were updated to it.

This is fallout from bug 653881, looks like.

In general, xbl:content and xbl:children are the "interesting" things.
Blocks: 653881
Flags: needinfo?(bzbarsky) → needinfo?(mrbkap)
Whiteboard: btpp-followup-2016-07-05
(In reply to Boris Zbarsky [:bz] (Out June 25-July 6) from comment #4)
> ChildIterator.cpp has explicit checks for xbl:children and assumes stuff if
> it sees them.  That's probably broken.  :(  They should probably be using
> IsActiveChildrenElement() instead or something.  That was added in bug
> 890775 but apparently not all consumers were updated to it.

ChildIterator.cpp does use IsActiveChildrenElement via nsContentUtils::IsContentInsertionPoint. The problem is that when we append an <xbl:children> to an active shadow root, we set its insertion parent, making it "active". We should probably detect when we're appending an <xbl:children> to a shadow root node and avoid setting its binding parent.
Flags: needinfo?(mrbkap) → needinfo?(wchen)
Flags: needinfo?(william) → needinfo?(mrbkap)
Priority: -- → P2
Whiteboard: btpp-followup-2016-07-05
Still reproducible on trunk. With Stylo enabled, the assertion is different, though.
Assertion failure: content->GetFlattenedTreeParentNodeForStyle() == &aContent, at z:/build/build/src/layout/base/ServoRestyleManager.cpp:1220
Has Regression Range: --- → no
Assignee: nobody → emilio
Flags: needinfo?(mrbkap)
Comment on attachment 8967819 [details]
Bug 1281745: Don't consider <xbl:children> in a shadow root without any binding active.

https://reviewboard.mozilla.org/r/236500/#review242298
Attachment #8967819 - Flags: review?(bugs) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6090c4592979
Don't consider <xbl:children> in a shadow root without any binding active. r=smaug
Comment on attachment 8967819 [details]
Bug 1281745: Don't consider <xbl:children> in a shadow root without any binding active.

https://reviewboard.mozilla.org/r/236500/#review242372

::: dom/base/nsIContentInlines.h:179
(Diff revision 1)
> +  nsIContent* bindingParent = GetBindingParent();
> +  if (!bindingParent) {
> +    return false;
> +  }
> +
> +  return !bindingParent->GetShadowRoot();

Please add a comment about why this is here. It's a bit surprising to see code talking about shadow roots when everything else is XBL-based.
Attachment #8967819 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/6090c4592979
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ac542a489af
bug 1453206: Add a comment pointing out why shadow root checks are needed. r=me
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.