Open Bug 1286419 Opened 8 years ago Updated 1 year ago

"ASSERTION: Reparenting something that has no usable parent? Shouldn't happen!" with createShadowRoot(), -moz-box


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




Tracking Status
firefox-esr52 --- disabled
firefox56 --- disabled
firefox57 --- disabled
firefox58 --- affected


(Reporter: jruderman, Unassigned)



(Keywords: assertion, testcase)


(3 files)

Attached file testcase
###!!! ASSERTION: Reparenting something that has no usable parent? Shouldn't happen!: 'newParentContext', file layout/base/RestyleManager.cpp, line 2334
Attached file stack
Component: DOM → Layout
Priority: -- → P3
Still happens on trunk with webcomponents preffed on and Stylo disabled.
Has Regression Range: --- → no
We're reparenting an inline whose mContent (a <span>) has a null GetFlattenedTreeParent().

It's not clear to me that the -moz-box is even relevant....

The only question is why we have a frame for the <span> at all, given that we don't think it's in the flattened tree.
The patches I'm working on for bug 1404789 should fix this (there's nothing right now ensuring frames go away when we change the distribution inside a shadow tree).
Depends on: 1404789
Still hits on autoland rev e214368792a2 (the push for bug 1404789) with or without Stylo enabled :(
Flags: needinfo?(emilio)
Hmmpf, ok, can look into this tomorrow morning and see what's going on... But, this assertion is Gecko-only, is the Stylo behavior the same? I doubt that.
Assignee: nobody → emilio
Flags: needinfo?(emilio) → needinfo?(ryanvm)
Flags: needinfo?(emilio)
The Stylo assertion:
Assertion failure: content->GetFlattenedTreeParentNodeForStyle() == &aContent, at z:/build/build/src/layout/base/ServoRestyleManager.cpp:1219
Flags: needinfo?(ryanvm)
This one is fun too.

Basically, the adoptNode call clears the Shadow DOM XBL binding, so we don't find it, and because of that, we think in FlattenedChildIterator that the content inside the element with the shadow root is part of the flat tree.

However, this check is inconsistent with what we do in GetFlattenedTreeParentNodeInternal (in particular, HasDistributedChildren returns true if GetShadowRoot returns non-null).

The patch above contains an assertion which we assume, basically, and that catches this bug.

Now, about possible fixes... They're not clear to me.

A workaround could be something like:

diff --git a/dom/base/ChildIterator.cpp b/dom/base/ChildIterator.cpp
index 985d667e4437..ee7bc23ba64e 100644
--- a/dom/base/ChildIterator.cpp
+++ b/dom/base/ChildIterator.cpp
@@ -143,8 +143,12 @@ FlattenedChildIterator::Init(bool aIgnoreXBL)
     mParent = binding->GetAnonymousContent();
     mXBLInvolved = true;
+  } else if (ShadowRoot* shadow = mParent->GetShadowRoot()) {
+    mParent = shadow;

But that feels like a hack, or at least shouldn't happen because we should always find the XBL binding.

At this point, it's not clear to me what should happen with the XBL binding when moving an element with a shadow root across documents.

I guess not clearing the XBL binding using the NODE_FORCE_XBL_BINDINGS flag as we do for XUL when adopting stuff could be reasonable...

Olli, wdyt?
Flags: needinfo?(emilio) → needinfo?(bugs)
(Or, maybe we should look for the bindings using GetComposedDoc instead of OwnerDoc...)
In any case this is a DOM bug. Happy to take it if we come up with a decent plan.
Assignee: emilio → nobody
Component: Layout → DOM: Core & HTML
Wit the patches from bug 1425759 there's no binding anymore.
Depends on: 1425759
Flags: needinfo?(bugs)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.