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

RESOLVED FIXED in Firefox 38

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: wilsonpage, Assigned: wchen)

Tracking

(Blocks: 1 bug)

unspecified
mozilla38
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
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/
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?
Note the spec bug at https://www.w3.org/Bugs/Public/show_bug.cgi?id=26943
(Reporter)

Comment 3

3 years ago
(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/
Yeah, sounds like the spec is bogus-as-usual, then.
(Reporter)

Comment 5

3 years ago
This could block progress in Gaia. Is it quick fix to get behaviour inline with Chrome?
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

3 years ago
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.)
(Reporter)

Comment 8

3 years ago
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();
}
Flags: needinfo?(bugs)

Comment 9

3 years ago
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.)
Flags: needinfo?(bugs) → needinfo?(wchen)
(Assignee)

Comment 10

3 years ago
Created attachment 8561922 [details] [diff] [review]
Part 1: Make GetComposedDoc() work in UnbindFromTree.
Assignee: nobody → wchen
Flags: needinfo?(wchen)
Attachment #8561922 - Flags: review?(bugs)
(Assignee)

Comment 11

3 years ago
Created attachment 8561923 [details] [diff] [review]
Part 2: Call attached and detached callback when attached and detached to/from the composed document.
Attachment #8561923 - Flags: review?(bugs)
Comment on attachment 8561922 [details] [diff] [review]
Part 1: Make GetComposedDoc() work in UnbindFromTree.

># HG changeset patch
># User William Chen <wchen@mozilla.com>
># 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?
Attachment #8561922 - Flags: review?(bugs) → review-

Updated

3 years ago
Attachment #8561923 - Flags: review?(bugs) → review+
(Assignee)

Comment 13

3 years ago
Created attachment 8562363 [details] [diff] [review]
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.
Attachment #8561922 - Attachment is obsolete: true
Attachment #8562363 - Flags: review?(bugs)
(Assignee)

Comment 14

3 years ago
Created attachment 8562364 [details] [diff] [review]
v1 diff v2 of part 1

Updated

3 years ago
Attachment #8562363 - Flags: review?(bugs) → review+
(Assignee)

Comment 15

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/09b002eaa381
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a7e30dbc96c
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/09b002eaa381
https://hg.mozilla.org/mozilla-central/rev/7a7e30dbc96c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.