1087460 - [shadow-dom][web-components] attachedCallback never fired on custom-element inside shadow-dom

Status: RESOLVED FIXED

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

Description

[Wilson Page [:wilsonpage]]

When a custom-element is inside the shadow-root of another custom-element the attached callback never fires.

TEST CASE

http://jsbin.com/wuwaxe/1/

Comment 1

[Boris Zbarsky [:bzbarsky]]

From the spec:

    Unless specified otherwise, this callback must be enqueued whenever custom element is
    inserted into a document and this document has a browsing context.

Things in that shadow DOM are never inserted into a document, so per spec this should never fire.

Does Chrome fire them?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Note the spec bug at https://www.w3.org/Bugs/Public/show_bug.cgi?id=26943

Comment 3

[Wilson Page [:wilsonpage]]

(In reply to Boris Zbarsky [:bz] from comment #1)
> From the spec:
> 
>     Unless specified otherwise, this callback must be enqueued whenever
> custom element is
>     inserted into a document and this document has a browsing context.
> 
> Things in that shadow DOM are never inserted into a document, so per spec
> this should never fire.
> 
> Does Chrome fire them?

Confirmed Chrome *does* fire them. If we don't get these callbacks it's not possible to use custom-elements inside the shadow-dom as we don't have to hooks required to setup once we have DOM context.

I added another button to example that proves this is when a custom-element exists within a shadow-root. Nothing to do with nested custom element.

http://jsbin.com/wuwaxe/2/

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, sounds like the spec is bogus-as-usual, then.

Comment 5

[Wilson Page [:wilsonpage]]

This could block progress in Gaia. Is it quick fix to get behaviour inline with Chrome?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Probably, once we figure out what Chrome's behavior actually is in various edge cases....  An exhaustive test of the possibilities would be somewhat helpful, I bet.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Yeah, it is not quite clear what the behavior should be for example in those older shadow dom
subtrees where host is in document, but younger shadow dom misses shadow insertion point.
I _think_ such older shadow dom subtree should behave as not being in document, but what does Chrome do? And does it do something sane?
(What Chrome currently does is against the spec in any case.)

Comment 8

[Wilson Page [:wilsonpage]]

Do we have any movement here? This is preventing web-component nesting. I'm currently doing a workaround where I manually call the attachedCallback:

attachedCallback: function() {
  nestedElement.attachedCallback();
}

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

The spec hasn't been fixed. https://www.w3.org/Bugs/Public/show_bug.cgi?id=26365 and related bugs like https://www.w3.org/Bugs/Public/show_bug.cgi?id=26943 are still open unfortunately.

I guess we could implement some behavior and then fix it later to work per spec,
once the spec if fixed.
(though it would be that we'd do against the current spec.)

Comment 10

[William Chen [:wchen]]

Attachment: Part 1: Make GetComposedDoc() work in UnbindFromTree.

Comment 11

[William Chen [:wchen]]

Attachment: Part 2: Call attached and detached callback when attached and detached to/from the composed document.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8561922 [details] [diff] [review]
Part 1: Make GetComposedDoc() work in UnbindFromTree.

># HG changeset patch
># User William Chen <[e-mail]>
># Date 1423504883 28800
>#      Mon Feb 09 10:01:23 2015 -0800
># Node ID 3bfdf12029cfe7e19f5cdeecc94a457150316abc
># Parent  2cb22c058add3ad58b68adae3ab06596fd4f723f
>Bug 1087460 - Part 1: Make GetComposedDoc() work in UnbindFromTree. r=NOT_REVIEWED
>
>diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp
>--- a/dom/base/Element.cpp
>+++ b/dom/base/Element.cpp
>@@ -987,31 +987,35 @@ Element::CreateShadowRoot(ErrorResult& a
>   protoBinding->SetInheritsStyle(false);
> 
>   // Calling SetPrototypeBinding takes ownership of protoBinding.
>   docInfo->SetPrototypeBinding(NS_LITERAL_CSTRING("shadowroot"), protoBinding);
> 
>   nsRefPtr<ShadowRoot> shadowRoot = new ShadowRoot(this, nodeInfo.forget(),
>                                                    protoBinding);
> 
>+  shadowRoot->SetIsComposedDocParticipant(!!GetComposedDoc());
I'd just use IsInComposedDoc(), not !!GetComposedDoc()

>   // Call BindToTree on shadow root children.
>   ShadowRoot* shadowRoot = GetShadowRoot();
>   if (shadowRoot) {
>+    shadowRoot->SetIsComposedDocParticipant(true);
Here you mark shadowroot to be in doc, even though the host might not be in document.
Perhaps shadowRoot->SetIsComposedDocParticipant(IsInComposedDoc());
(We call SetInDocument() higher up in the method)

>+
>+  // Flag to indicate whether the descendants of this shadow root are part of the
>+  // composed document. Ideally, we would use a node flag on nodes to
>+  // mark whether it is in the composed document, but we have run out of flags
>+  // so instead we track it here.
>+  bool mIsComposedDocParticipant;
In theory you could use NODE_TYPE_SPECIFIC_BITS_OFFSET for ShadowRoot, in which case 
mInsertionPointChanged should use similar flag too, but the patch shouldn't make ShadowRoot larger than it is now, so feel free to
switch to use NODE_TYPE_SPECIFIC_BITS_OFFSET in a different bug.

>   if (mIsInsertionPoint && containingShadow) {
>     // Propagate BindToTree calls to projected shadow root children.
>     ShadowRoot* projectedShadow = containingShadow->GetOlderShadowRoot();
>     if (projectedShadow) {
>+      projectedShadow->SetIsComposedDocParticipant(true);
Why is this right?

Comment 13

[William Chen [:wchen]]

Attachment: Part 1: Make GetComposedDoc() work in UnbindFromTree. v2

(In reply to Olli Pettay [:smaug] from comment #12)
> Comment on attachment 8561922 [details] [diff] [review]
> Part 1: Make GetComposedDoc() work in UnbindFromTree.
> >   if (mIsInsertionPoint && containingShadow) {
> >     // Propagate BindToTree calls to projected shadow root children.
> >     ShadowRoot* projectedShadow = containingShadow->GetOlderShadowRoot();
> >     if (projectedShadow) {
> >+      projectedShadow->SetIsComposedDocParticipant(true);
> Why is this right?

It's not, it has the same problem you pointed out in the other spots.

Comment 14

[William Chen [:wchen]]

Attachment: v1 diff v2 of part 1

Comment 15

[William Chen [:wchen]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/09b002eaa381
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a7e30dbc96c

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/09b002eaa381
https://hg.mozilla.org/mozilla-central/rev/7a7e30dbc96c