Closed Bug 1281745 Opened 4 years ago Closed 2 years ago

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

Categories

(Core :: DOM: Core & HTML, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: jruderman, Assigned: emilio)

References

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file testcase
Assertion failure: insertion.mContainer == GetInsertionPoint(parent, child).mContainer, at layout/base/nsCSSFrameConstructor.cpp:10671
Attached 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: 2 years ago
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
Flags: in-testsuite+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.