Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose shadowRoot on element internals #871

Closed
JanMiksovsky opened this issue Mar 24, 2020 · 59 comments
Closed

Expose shadowRoot on element internals #871

JanMiksovsky opened this issue Mar 24, 2020 · 59 comments

Comments

@JanMiksovsky
Copy link

JanMiksovsky commented Mar 24, 2020

During the March 2020 web components virtual F2F, we discussed the fact that an element with a closed shadow root has no standard way to refer to its own shadow root:

class MyElement extends HTMLElement {
  constructor() {
    super();
    this.attachShadow({ mode: "closed" });
  }

  connectedCallback() {
    // Other callbacks/methods can't access shadow.
    this.shadowRoot.innerHTML = "Hello"; // Throws: Can't set innerHTML of null
  }
}

Because of this, any component that creates a closed root must maintain its own private reference to the root.

Additionally, the possibility of declarative Shadow DOM introduces a scenario in which a rehydrated component may need access to a shadow root that was automatically created for it.

For these reasons, it seems worthwhile to expose shadowRoot on an element's internals:

class MyElement extends HTMLElement {
  #internals;

  constructor() {
    super();
    this.attachShadow({ mode: "closed" });
    this.#internals = this.attachInternals();
  }

  connectedCallback() {
    // Other callbacks/methods can access shadow via internals.
    this.#internals.shadowRoot.innerHTML = "Hello";
  }
}

As before, the component must still maintain a private reference (to internals instead of the shadow root), but at least now:

  • A component that wants to have both a closed shadow root and internals has to track only a reference to the internals. Through that, it can always obtain a reference to the shadowRoot.
  • A component with declarative Shadow DOM could invoke attachInternals and then inspect the internals to determine whether a shadow root has already been created.
@rniwa
Copy link
Collaborator

rniwa commented Mar 25, 2020

We (Apple's WebKit team) initially suggested two alternatives to this approach:

  • attachShadow during custom element construction replaces the declarative shadow DOM
  • attachShadow during custom element constructor returns the declarative Shadow DOM instead of creating one.

We'll review this alternative approach (exposing shadow root on ElementInternals).

@caridy
Copy link

caridy commented Mar 25, 2020

@rniwa

attachShadow during custom element constructor returns the declarative Shadow DOM instead of creating one.

that seems a lot more complicated, Element.prototype.shadowRoot getter will return the instance during construction, and return null after the construction phase when declarative is closed. Sounds like hazardous for developers.

@rniwa
Copy link
Collaborator

rniwa commented Mar 26, 2020

@rniwa

attachShadow during custom element constructor returns the declarative Shadow DOM instead of creating one.

that seems a lot more complicated, Element.prototype.shadowRoot getter will return the instance during construction, and return null after the construction phase when declarative is closed. Sounds like hazardous for developers.

That's not at all what I'm suggesting. I'm suggesting that attachShadow would return the existing declarative shadow root instead of creating a new one.

@caridy
Copy link

caridy commented Mar 26, 2020

@rniwa During Mason's presentation, we mentioned that the problem with this approach is that it is not backward compatible, components that are not taking declarative shadow root into consideration, will fail because they will assume an empty shadowRoot was created.

@justinfagnani
Copy link
Contributor

@caridy I don't see how that will ever happen. A component will necessarily have to cooperate to produce a declarative version of its shadow root. How else would it get a valid one?

@caridy
Copy link

caridy commented Mar 26, 2020

@justinfagnani as we discussed in the meeting, we believe that it should be backward compatible, letting existing components and simple components to function even when used through SSR via declarative shadow root. Yes, the component will wipe out the existing shadow if the component doesn't take it into consideration, that's fine. @mfreed7 agreed with that IIRC. We should probably have this issue in the declarative shadow root proposal repo, I don't think that decision affects this request for element internals to expose the shadow root.

@justinfagnani
Copy link
Contributor

@caridy can you outline how a component would even get a valid shadow root generated for it without cooperation from the element or framework it's built in? It affects this feature request because creating a new shadow root is a very new behavior.

@mfreed7
Copy link

mfreed7 commented Mar 26, 2020

Thanks @JanMiksovsky for opening this issue. It seems that there are two relevant potential use cases here:

  1. Existing components, built without knowledge of declarative Shadow DOM. These call this.attachShadow() and assume a. it does not throw, and b. it returns an empty shadow root. It then fills the shadowroot with shadow content.
  2. Components that are built with knowledge of declarative Shadow DOM, and that wish to use a closed shadow root. In this case, the component will want to retrieve the existing (closed) shadowRoot so that it can verify the content and/or hook up events and references. It does not want to blow away and re-create existing declarative content.

It would seem that the most clean API to address both cases above would be:

  1. For use case Add index.html in the root directory to make specifications more discoverable #1, make attachShadow() blow away any existing declaratively-created shadow root, and return a new, empty shadowroot. (Alternatively, this could delete the existing contents of the shadowroot, and return a reference to the existing shadowroot.)
  2. For use case Add link to Templates spec #2, add a shadowRoot accessor to ElementInternals, to allow "declarative aware" components to retrieve their closed shadow root. Alternatively, an extra argument could be added to attachShadow() which causes it to retrieve the existing shadow root without removing its contents. Either would seem to achieve the goal. Though ElementInternals.shadowRoot does seem considerably better, ergonomically.

@justinfagnani, I could imagine a framework that simply renders all content server-side, and then calls getInnerHTML() to retrieve the declarative content to send down to the client. In this case, the component and the framework would not need to "coordinate", and you could then get use case #1 above that wakes up with declarative content but doesn't know how to deal with it.

@rniwa
Copy link
Collaborator

rniwa commented Mar 26, 2020

It seems awkward that all SSR aware components would have to write code like this:

class CustomHTMLElement extends HTMLElement {
    #internals;

    constructor() {
        super();
        #internals = this.attachInternals();
        if (!#internals.shadowRoot)
            this.attachShadow({mode: 'closed'});
    }
}

With the alternative I'm proposing (make attachShadow return the exiting declarative shadowRoot), we could something like this (obviously with a shorter name):

class CustomHTMLElement extends HTMLElement {
    #shadowRoot;

    constructor() {
        super();
        #shadowRoot = this.attachShadow({mode: 'closed', returnExistingDeclarativeShadowRoot: true});
    }
}

We could do both too.

@justinfagnani
Copy link
Contributor

Many templating libraries that support hydration will still need to branch on some factor to either call the initial rendering or hydrating entrypoint. The existence of a shadow root is a pretty decent signal for this, IMO.

@mfreed7
Copy link

mfreed7 commented Mar 26, 2020

@rniwa , just to double-check your proposal:

#shadowRoot = this.attachShadow({mode: 'closed', returnExistingDeclarativeShadowRoot: true});

will return the existing declarative shadow root, with contents included.

#shadowRoot = this.attachShadow({mode: 'closed'});

will instead blow away the existing contents of the shadow root and return an empty root. In both cases, no exception will be thrown if a declarative shadow root already exists. However, if there is already an imperatively-created shadow root, these will both throw, as they do today.

Is that right? If so, I think I'm ok with that proposal also.

@rniwa
Copy link
Collaborator

rniwa commented Mar 27, 2020

In both cases, no exception will be thrown if a declarative shadow root already exists. However, if there is already an imperatively-created shadow root, these will both throw, as they do today.

Right.

Many templating libraries that support hydration will still need to branch on some factor to either call the initial rendering or hydrating entrypoint. The existence of a shadow root is a pretty decent signal for this, IMO.

Now I realize the situation is a lot more complicated than that because of the sync parsing behavior in which case the custom element gets created before any of its children get parsed. In that scenario (non-upgrading case), you'd have to wait until all the children had been parsed for hydration.

@justinfagnani
Copy link
Contributor

justinfagnani commented Mar 27, 2020

In that scenario (non-upgrading case), you'd have to wait until all the children had been parsed for hydration.

With the current declarative SR proposal, this is fine, as the entire shadow root is created when all the shadow children are parsed.

If we found an acceptable way to do streaming, this would be an issue to tackle too.

Edit: I suppose the constructor could be called before the declarative shadow root is created at all. This would be a great time to add the childrenParsed or whatever callback. That's a problem that custom element authors have been hitting for a while now.

@rniwa
Copy link
Collaborator

rniwa commented Mar 27, 2020

In that scenario (non-upgrading case), you'd have to wait until all the children had been parsed for hydration.

With the current declarative SR proposal, this is fine, as the entire shadow root is created when all the shadow children are parsed.

No, it's not fine because the constructor of a custom element is always called before its children are parsed or created so in sync/non-upgrade construction case, the constructor will always run and then it can attach a non-declarative shadow root.

If we found an acceptable way to do streaming, this would be an issue to tackle too.

Edit: I suppose the constructor could be called before the declarative shadow root is created at all.

This is not could. It's guaranteed to happen in that order.

This would be a great time to add the childrenParsed or whatever callback. That's a problem that custom element authors have been hitting for a while now.

That doesn't necessarily solve the problem fully because then the custom element has to delay attaching a shadow root until that happens in the case there is a declarative shadow root, in which case, someone else can attach a shadow root, or worse yet, anyone can access to template's content and have reference to those nodes.

@justinfagnani
Copy link
Contributor

in which case, someone else can attach a shadow root, or worse yet, anyone can access to template's content and have reference to those nodes.

Declarative shadow roots are necessarily a cooperative construct - the generator of the shadow root and the element have to coordinate - and I'm definitely not worried about breaking encapsulation this way. A component will have to opt-in to SSR somehow and have to trust the declarative SR contents. If a component author is worried that there will somehow be an attack via declarative SR, then they should not support it, ie, it should throw when they call attachShadow as it does now.

This is why I don't see a forgiving version of attachShadow as necessary. The component should only call that if it knows it's not going to receive a declarative SR.

@rniwa
Copy link
Collaborator

rniwa commented Mar 27, 2020

in which case, someone else can attach a shadow root, or worse yet, anyone can access to template's content and have reference to those nodes.

Declarative shadow roots are necessarily a cooperative construct - the generator of the shadow root and the element have to coordinate - and I'm definitely not worried about breaking encapsulation this way.

Breaking encapsulation in this scenario would definitely be a show stopper for us.

A component will have to opt-in to SSR somehow and have to trust the declarative SR contents. If a component author is worried that there will somehow be an attack via declarative SR, then they should not support it, ie, it should throw when they call attachShadow as it does now.

This is not about the malicious case of something attacking since there is no security boundary right now. However, it's very easy for some residual script on a page to start accessing random nodes via MutationObserver, etc...

@mfreed7
Copy link

mfreed7 commented Apr 2, 2020

I've just posted a larger comment on the 831 issue, which should be read before this one for context. But I wanted to point out one problem with the attachShadow({mode: 'closed', returnExistingDeclarativeShadowRoot: true}); version of this feature. That will not work for the case where there is an imperative/declarative race, and the imperative code "wins". In that case, the component will likely want to detect that the declarative content hasn't loaded, and wait until later to hydrate. But there will be no way to detect that situation, since calling attachShadow() will actually attach a shadow root and block the declarative content. If, on the other hand, we go with an ElementInternals.shadowRoot accessor, this situation can be properly detected no matter who wins the race.

@mfreed7
Copy link

mfreed7 commented Aug 11, 2020

I would like to re-iterate my comment, and propose that we solve this issue by adding the following to the ElementInternals interface:


interface ElementInternals {
  readonly attribute ShadowRoot? shadowRoot;
};

internals.shadowRoot: Returns internals's target element's shadow root. This attribute will return both open and closed shadow roots. If shadow is null, then return null.


If there is general agreement on this, then I will put together a spec PR, and will get this implemented in Chromium.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 11, 2020
See [1] for context. This CL adds the shadowRoot attribute to ElementInternals,
which allows custom elements to access their own shadow roots, including in
the case that the shadowRoot is closed. Without this capability, custom
elements that use closed declarative shadow roots would have no ability to
retrieve the contents of their own shadow root.

Bug: 1042130

[1] WICG/webcomponents#871 (comment)

Change-Id: I33606144bec3c27600d2e67f108d0344f7696dd2
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 12, 2020
See [1] for context. This CL adds the shadowRoot attribute to ElementInternals,
which allows custom elements to access their own shadow roots, including in
the case that the shadowRoot is closed. Without this capability, custom
elements that use closed declarative shadow roots would have no ability to
retrieve the contents of their own shadow root.

Bug: 1042130

[1] WICG/webcomponents#871 (comment)

Change-Id: I33606144bec3c27600d2e67f108d0344f7696dd2
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 13, 2020
See [1] for context. This CL adds the shadowRoot attribute to ElementInternals,
which allows custom elements to access their own shadow roots, including in
the case that the shadowRoot is closed. Without this capability, custom
elements that use closed declarative shadow roots would have no ability to
retrieve the contents of their own shadow root.

Bug: 1042130

[1] WICG/webcomponents#871 (comment)

Change-Id: I33606144bec3c27600d2e67f108d0344f7696dd2
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 13, 2020
See [1] for context. This CL adds the shadowRoot attribute to ElementInternals,
which allows custom elements to access their own shadow roots, including in
the case that the shadowRoot is closed. Without this capability, custom
elements that use closed declarative shadow roots would have no ability to
retrieve the contents of their own shadow root.

Bug: 1042130

[1] WICG/webcomponents#871 (comment)

Change-Id: I33606144bec3c27600d2e67f108d0344f7696dd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2350739
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/master@{#797729}
@mfreed7
Copy link

mfreed7 commented Aug 13, 2020

To allow testing, I've implemented this in Chromium behind the DeclarativeShadowDOM flag, with WPT tests. Very straightforward implementation, and I believe it solves the issues brought up here. But comments appreciated.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 15, 2020
Per the discussion at [1], the intention of this change is to prevent
calls to attachInternals() prior to the constructor of the custom
element having a chance to do so. The spec PR is at [2].

This change is gated behind the DeclarativeShadowDOM flag. With the
flag disabled (the default), a use counter is added for checking on
the web compatibility of this change. The use counter will measure
the cases where attachInternals() is being called in a to-be-outlawed
way.

[1] WICG/webcomponents#871 (comment)
[2] whatwg/html#5909

Bug: 1042130
Change-Id: Iacf97a49133b5f7f44710e5c0287f01cfebe4c44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392975
Reviewed-by: Alexei Svitkine <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/master@{#806830}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 15, 2020
Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
@mfreed7
Copy link

mfreed7 commented Sep 15, 2020

With that amendment, it seems like declarative Shadow DOM would be compatible with closed shadow trees so we can assume this technical issue has been resolved.

Great. I've added two more spec PRs for this change, one in HTML and one in DOM. This makes four open PRs here, which could use reviews:

  1. (HTML) Prevent attachInternals() use before custom element constructor
  2. (HTML) Add ElementInternals.shadowRoot
  3. (HTML) Prevent access to ElementInternals.shadowRoot for pre-existing shadow roots
  4. (DOM) Add is-available-to-element-internals

There is still a pending question of whether we support the declarative Shadow DOM at conceptual level. I need to get back to you on that.

Ok, thanks for the update. I would really appreciate your thoughts soon - we are proceeding with an Origin Trial as we speak, and I'd love to ship this feature soon. I would prefer not to do that without your input and support. As I mentioned in the main thread, I believe the currently-proposed solution is worthwhile, for reasons other than pure performance.

blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Sep 16, 2020
Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807311}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 16, 2020
Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807311}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 16, 2020
Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807311}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 23, 2020
…ide or after the constructor, a=testonly

Automatic update from web-platform-tests
Restrict attachInternals() to be run inside or after the constructor

Per the discussion at [1], the intention of this change is to prevent
calls to attachInternals() prior to the constructor of the custom
element having a chance to do so. The spec PR is at [2].

This change is gated behind the DeclarativeShadowDOM flag. With the
flag disabled (the default), a use counter is added for checking on
the web compatibility of this change. The use counter will measure
the cases where attachInternals() is being called in a to-be-outlawed
way.

[1] WICG/webcomponents#871 (comment)
[2] whatwg/html#5909

Bug: 1042130
Change-Id: Iacf97a49133b5f7f44710e5c0287f01cfebe4c44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392975
Reviewed-by: Alexei Svitkine <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/master@{#806830}

--

wpt-commits: df5b354d012f3bb0db1524bfaf8a4e16b4b01cc2
wpt-pr: 25402
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 23, 2020
…-created shadow roots, a=testonly

Automatic update from web-platform-tests
Deny ElementInternals.shadowRoot for pre-created shadow roots

Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807311}

--

wpt-commits: 8e3392df536283e843589c176ffbc7e00e94e64e
wpt-pr: 25554
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Sep 24, 2020
…ide or after the constructor, a=testonly

Automatic update from web-platform-tests
Restrict attachInternals() to be run inside or after the constructor

Per the discussion at [1], the intention of this change is to prevent
calls to attachInternals() prior to the constructor of the custom
element having a chance to do so. The spec PR is at [2].

This change is gated behind the DeclarativeShadowDOM flag. With the
flag disabled (the default), a use counter is added for checking on
the web compatibility of this change. The use counter will measure
the cases where attachInternals() is being called in a to-be-outlawed
way.

[1] WICG/webcomponents#871 (comment)
[2] whatwg/html#5909

Bug: 1042130
Change-Id: Iacf97a49133b5f7f44710e5c0287f01cfebe4c44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392975
Reviewed-by: Alexei Svitkine <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/master@{#806830}

--

wpt-commits: df5b354d012f3bb0db1524bfaf8a4e16b4b01cc2
wpt-pr: 25402
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Sep 24, 2020
…-created shadow roots, a=testonly

Automatic update from web-platform-tests
Deny ElementInternals.shadowRoot for pre-created shadow roots

Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807311}

--

wpt-commits: 8e3392df536283e843589c176ffbc7e00e94e64e
wpt-pr: 25554
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Sep 28, 2020
…ide or after the constructor, a=testonly

Automatic update from web-platform-tests
Restrict attachInternals() to be run inside or after the constructor

Per the discussion at [1], the intention of this change is to prevent
calls to attachInternals() prior to the constructor of the custom
element having a chance to do so. The spec PR is at [2].

This change is gated behind the DeclarativeShadowDOM flag. With the
flag disabled (the default), a use counter is added for checking on
the web compatibility of this change. The use counter will measure
the cases where attachInternals() is being called in a to-be-outlawed
way.

[1] WICG/webcomponents#871 (comment)
[2] whatwg/html#5909

Bug: 1042130
Change-Id: Iacf97a49133b5f7f44710e5c0287f01cfebe4c44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392975
Reviewed-by: Alexei Svitkine <asvitkinechromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Commit-Queue: Mason Freed <masonfreedchromium.org>
Auto-Submit: Mason Freed <masonfreedchromium.org>
Cr-Commit-Position: refs/heads/master{#806830}

--

wpt-commits: df5b354d012f3bb0db1524bfaf8a4e16b4b01cc2
wpt-pr: 25402

UltraBlame original commit: 56afece076c057fd659c3c180e1673c5a409a614
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Sep 28, 2020
…-created shadow roots, a=testonly

Automatic update from web-platform-tests
Deny ElementInternals.shadowRoot for pre-created shadow roots

Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Auto-Submit: Mason Freed <masonfreedchromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Cr-Commit-Position: refs/heads/master{#807311}

--

wpt-commits: 8e3392df536283e843589c176ffbc7e00e94e64e
wpt-pr: 25554

UltraBlame original commit: 11691695d1b65041eea7dd2470183b95f4335029
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Sep 28, 2020
…ide or after the constructor, a=testonly

Automatic update from web-platform-tests
Restrict attachInternals() to be run inside or after the constructor

Per the discussion at [1], the intention of this change is to prevent
calls to attachInternals() prior to the constructor of the custom
element having a chance to do so. The spec PR is at [2].

This change is gated behind the DeclarativeShadowDOM flag. With the
flag disabled (the default), a use counter is added for checking on
the web compatibility of this change. The use counter will measure
the cases where attachInternals() is being called in a to-be-outlawed
way.

[1] WICG/webcomponents#871 (comment)
[2] whatwg/html#5909

Bug: 1042130
Change-Id: Iacf97a49133b5f7f44710e5c0287f01cfebe4c44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392975
Reviewed-by: Alexei Svitkine <asvitkinechromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Commit-Queue: Mason Freed <masonfreedchromium.org>
Auto-Submit: Mason Freed <masonfreedchromium.org>
Cr-Commit-Position: refs/heads/master{#806830}

--

wpt-commits: df5b354d012f3bb0db1524bfaf8a4e16b4b01cc2
wpt-pr: 25402

UltraBlame original commit: 56afece076c057fd659c3c180e1673c5a409a614
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Sep 28, 2020
…-created shadow roots, a=testonly

Automatic update from web-platform-tests
Deny ElementInternals.shadowRoot for pre-created shadow roots

Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Auto-Submit: Mason Freed <masonfreedchromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Cr-Commit-Position: refs/heads/master{#807311}

--

wpt-commits: 8e3392df536283e843589c176ffbc7e00e94e64e
wpt-pr: 25554

UltraBlame original commit: 11691695d1b65041eea7dd2470183b95f4335029
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Sep 28, 2020
…ide or after the constructor, a=testonly

Automatic update from web-platform-tests
Restrict attachInternals() to be run inside or after the constructor

Per the discussion at [1], the intention of this change is to prevent
calls to attachInternals() prior to the constructor of the custom
element having a chance to do so. The spec PR is at [2].

This change is gated behind the DeclarativeShadowDOM flag. With the
flag disabled (the default), a use counter is added for checking on
the web compatibility of this change. The use counter will measure
the cases where attachInternals() is being called in a to-be-outlawed
way.

[1] WICG/webcomponents#871 (comment)
[2] whatwg/html#5909

Bug: 1042130
Change-Id: Iacf97a49133b5f7f44710e5c0287f01cfebe4c44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392975
Reviewed-by: Alexei Svitkine <asvitkinechromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Commit-Queue: Mason Freed <masonfreedchromium.org>
Auto-Submit: Mason Freed <masonfreedchromium.org>
Cr-Commit-Position: refs/heads/master{#806830}

--

wpt-commits: df5b354d012f3bb0db1524bfaf8a4e16b4b01cc2
wpt-pr: 25402

UltraBlame original commit: 56afece076c057fd659c3c180e1673c5a409a614
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Sep 28, 2020
…-created shadow roots, a=testonly

Automatic update from web-platform-tests
Deny ElementInternals.shadowRoot for pre-created shadow roots

Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <kouheichromium.org>
Auto-Submit: Mason Freed <masonfreedchromium.org>
Reviewed-by: Kouhei Ueno <kouheichromium.org>
Cr-Commit-Position: refs/heads/master{#807311}

--

wpt-commits: 8e3392df536283e843589c176ffbc7e00e94e64e
wpt-pr: 25554

UltraBlame original commit: 11691695d1b65041eea7dd2470183b95f4335029
domenic pushed a commit to whatwg/dom that referenced this issue Oct 5, 2020
This is for use with whatwg/html#5912, which
restricts access to ElementInternals's shadowRoot property for shadow
roots which were pre-existing before a custom element
upgrade/construction. See the discussion in
WICG/webcomponents#871 (comment)
for more context.
@mfreed7
Copy link

mfreed7 commented Oct 5, 2020

Thanks for the PR reviews here, I'm glad we could put together a solution to this problem.

While this is closed, I will continue to monitor the use counters I recently added for usage of the attachInternals() API prior to the constructor running, and overall attachInternals() usage at any time. The early numbers are very encouraging in terms of compat: I don't see any usage yet for the pre-constructor syntax that is now outlawed. The new spec is implemented in Chromium behind the Experimental Web Platform Features flag, but at this point I don't see any barriers to bringing it out from behind the flag.

ambroff pushed a commit to ambroff/gecko that referenced this issue Nov 4, 2020
…turns both open and closed roots, a=testonly

Automatic update from web-platform-tests
Add ElementInternals.shadowRoot which returns both open and closed roots

See [1] for context. This CL adds the shadowRoot attribute to ElementInternals,
which allows custom elements to access their own shadow roots, including in
the case that the shadowRoot is closed. Without this capability, custom
elements that use closed declarative shadow roots would have no ability to
retrieve the contents of their own shadow root.

Bug: 1042130

[1] WICG/webcomponents#871 (comment)

Change-Id: I33606144bec3c27600d2e67f108d0344f7696dd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2350739
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/master@{#797729}

--

wpt-commits: 9734707a86c1a14cba63bb80d8c391dd7dea004f
wpt-pr: 24969
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
See [1] for context. This CL adds the shadowRoot attribute to ElementInternals,
which allows custom elements to access their own shadow roots, including in
the case that the shadowRoot is closed. Without this capability, custom
elements that use closed declarative shadow roots would have no ability to
retrieve the contents of their own shadow root.

Bug: 1042130

[1] WICG/webcomponents#871 (comment)

Change-Id: I33606144bec3c27600d2e67f108d0344f7696dd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2350739
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#797729}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 9e291a961dbb34688f539a0ad10bd3cb3874a61a
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Per the discussion at [1], the intention of this change is to prevent
calls to attachInternals() prior to the constructor of the custom
element having a chance to do so. The spec PR is at [2].

This change is gated behind the DeclarativeShadowDOM flag. With the
flag disabled (the default), a use counter is added for checking on
the web compatibility of this change. The use counter will measure
the cases where attachInternals() is being called in a to-be-outlawed
way.

[1] WICG/webcomponents#871 (comment)
[2] whatwg/html#5909

Bug: 1042130
Change-Id: Iacf97a49133b5f7f44710e5c0287f01cfebe4c44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2392975
Reviewed-by: Alexei Svitkine <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/master@{#806830}
GitOrigin-RevId: 8f4d32caa68b363efdb4cd27821f3d893ff6f464
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Per the spec issue [1], this change disallows the use of
ElementInternals.shadowRoot for shadow roots created prior
to the custom element constructor being run. This protects
potentially-closed shadow roots, created outside of the
custom element, from being revealed to the custom element
itself. (Use case unclear, but this was the request.)

[1] WICG/webcomponents#871

Bug: 1042130
Change-Id: I25192256e8b1334d09ea587f29d64f35d4f5f949
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412470
Commit-Queue: Kouhei Ueno <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/master@{#807311}
GitOrigin-RevId: 2372aae3a33f3928c0964defb4f01b9be77ef3ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants