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

Status: NEW

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

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Reparenting something that has no usable parent? Shouldn't happen!: 'newParentContext', file layout/base/RestyleManager.cpp, line 2334

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Still happens on trunk with webcomponents preffed on and Stylo disabled.

Comment 3

[Boris Zbarsky [:bzbarsky]]

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.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

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).

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Still hits on autoland rev e214368792a2 (the push for bug 1404789) with or without Stylo enabled :(

Comment 6

[Emilio Cobos Álvarez (: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.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

The Stylo assertion:
Assertion failure: content->GetFlattenedTreeParentNodeForStyle() == &aContent, at z:/build/build/src/layout/base/ServoRestyleManager.cpp:1219

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Assertion that should hold as things stand, but doesn't.

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)
     MOZ_ASSERT(binding->GetAnonymousContent());
     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?

Comment 9

[Emilio Cobos Álvarez (:emilio)]

(Or, maybe we should look for the bindings using GetComposedDoc instead of OwnerDoc...)

Comment 10

[Emilio Cobos Álvarez (:emilio)]

In any case this is a DOM bug. Happy to take it if we come up with a decent plan.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Wit the patches from bug 1425759 there's no binding anymore.