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

Add shadowrootclonable and make declarative shadow roots not clonable by default #10117

Merged
merged 3 commits into from
Feb 13, 2024

Conversation

mfreed7
Copy link
Contributor

@mfreed7 mfreed7 commented Feb 2, 2024

(See WHATWG Working Mode: Changes for more details.)

@annevk @emilio


/indices.html ( diff )
/infrastructure.html ( diff )
/parsing.html ( diff )
/scripting.html ( diff )

…ble` by default

- [ ] At least two implementers are interested (and none opposed):
   * Chromium
   * [WebKit](whatwg#10107 (comment))
   * [Gecko](whatwg#10107 (comment))
- [X] [Tests](https://github.com/web-platform-tests/wpt) are written and can be reviewed and commented upon at:
   * https://wpt.fyi/results/shadow-dom/shadow-root-clonable.html
- [X] [Implementation bugs](https://github.com/whatwg/meta/blob/main/MAINTAINERS.md#handling-pull-requests) are filed:
   * Chromium: https://crbug.com/1510466
   * Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1868428
   * WebKit: https://bugs.webkit.org/show_bug.cgi?id=266227
- [ ] [MDN issue](https://github.com/whatwg/meta/blob/main/MAINTAINERS.md#handling-pull-requests) is filed: …
- [X] The top of this comment includes a [clear commit message](https://github.com/whatwg/meta/blob/main/COMMITTING.md) to use.

(See [WHATWG Working Mode: Changes](https://whatwg.org/working-mode#changes) for more details.)

@annevk @emilio
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 2, 2024
See the discussion here:

whatwg/html#10107 (comment)

The new consensus is that the old behavior was likely web-
incompatible, plus not very developer-desirable. The new behavior
adds a `shadowrootclonable` attribute for declarative shadow dom
to opt-in to clonable shadow roots.

This is a slight behavior change from the existing shipped
behavior, in that before the `clonable` concept was introduced,
*any* declarative shadow root within a `<template>` would be
cloned. Now, that behavior is opt in. So:

old:
  <template>
    <div>
      <template shadowrootmode=open>
        I do NOT get cloned!
      </template>
    </div>
  </template>

new:
  <template>
    <div>
      <template shadowrootmode=open shadowrootclonable>
        I get cloned!
      </template>
    </div>
  </template>

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 2, 2024
See the discussion here:

whatwg/html#10107 (comment)

The new consensus is that the old behavior was likely web-
incompatible, plus not very developer-desirable. The new behavior
adds a `shadowrootclonable` attribute for declarative shadow dom
to opt-in to clonable shadow roots.

This is a slight behavior change from the existing shipped
behavior, in that before the `clonable` concept was introduced,
*any* declarative shadow root within a `<template>` would be
cloned. Now, that behavior is opt in. So:

old:
  <template>
    <div>
      <template shadowrootmode=open>
        I do NOT get cloned!
      </template>
    </div>
  </template>

new:
  <template>
    <div>
      <template shadowrootmode=open shadowrootclonable>
        I get cloned!
      </template>
    </div>
  </template>

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I pushed a number of nits. Please review.

I actually want to more substantially change the HTML parser "template" start tag section to have a numbered list and use camel-cased variables throughout, but I'll wait with that until we land this and the other PR.

@smaug---- @rniwa @hsivonen I plan to merge this Monday but feel free to ask for more time.

@mfreed7
Copy link
Contributor Author

mfreed7 commented Feb 2, 2024

Thanks!

I pushed a number of nits. Please review.

Thanks for the cleanup - all of them look good to me!

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 2, 2024
See the discussion here:

whatwg/html#10107 (comment)

The new consensus is that the old behavior was likely web-
incompatible, plus not very developer-desirable. The new behavior
adds a `shadowrootclonable` attribute for declarative shadow dom
to opt-in to clonable shadow roots.

This is a slight behavior change from the existing shipped
behavior, in that before the `clonable` concept was introduced,
*any* declarative shadow root within a `<template>` would be
cloned. Now, that behavior is opt in. So:

old:
  <template>
    <div>
      <template shadowrootmode=open>
        I do NOT get cloned!
      </template>
    </div>
  </template>

new:
  <template>
    <div>
      <template shadowrootmode=open shadowrootclonable>
        I get cloned!
      </template>
    </div>
  </template>

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 3, 2024
See the discussion here:

whatwg/html#10107 (comment)

The new consensus is that the old behavior was likely web-
incompatible, plus not very developer-desirable. The new behavior
adds a `shadowrootclonable` attribute for declarative shadow dom
to opt-in to clonable shadow roots.

This is a slight behavior change from the existing shipped
behavior, in that before the `clonable` concept was introduced,
*any* declarative shadow root within a `<template>` would be
cloned. Now, that behavior is opt in. So:

old:
  <template>
    <div>
      <template shadowrootmode=open>
        I do NOT get cloned!
      </template>
    </div>
  </template>

new:
  <template>
    <div>
      <template shadowrootmode=open shadowrootclonable>
        I get cloned!
      </template>
    </div>
  </template>

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
@annevk
Copy link
Member

annevk commented Feb 5, 2024

Given the discussion in #10107 I suppose I need to give this at least one more day for people to respond.

@domenic domenic added normative change addition/proposal New features or enhancements topic: shadow Relates to shadow trees (as defined in DOM) labels Feb 6, 2024
@domenic
Copy link
Member

domenic commented Feb 6, 2024

Drive-by: Did we already discuss "cloneable" vs. "clonable" (no "e")? I guess shadowRoot.clonable shipped some time ago, so I'm codifying that in whatwg/whatwg.org#433.

Some interesting links I found:

@annevk
Copy link
Member

annevk commented Feb 6, 2024

There was no discussion that I recall. I do remember checking if the spelling was used, but then also missing WebKit had the "incorrect" spelling for a while. I'm fine with it though.

@mfreed7
Copy link
Contributor Author

mfreed7 commented Feb 6, 2024

Drive-by: Did we already discuss "cloneable" vs. "clonable" (no "e")? I guess shadowRoot.clonable shipped some time ago, so I'm codifying that in whatwg/whatwg.org#433.

It's weirdly hard to tell what the actual "correct" spelling is for this word, since the discourse is corrupted by the computing references (like Java). It sounds vaguely to me like clonable might be the proper English. In any case, I'm really glad it looks like we don't have to rename this. 😄

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 7, 2024
See the discussion here:

whatwg/html#10107 (comment)

The new consensus is that the old behavior was likely web-
incompatible, plus not very developer-desirable. The new behavior
adds a `shadowrootclonable` attribute for declarative shadow dom
to opt-in to clonable shadow roots.

This is a slight behavior change from the existing shipped
behavior, in that before the `clonable` concept was introduced,
*any* declarative shadow root within a `<template>` would be
cloned. Now, that behavior is opt in. So:

old:
  <template>
    <div>
      <template shadowrootmode=open>
        I do NOT get cloned!
      </template>
    </div>
  </template>

new:
  <template>
    <div>
      <template shadowrootmode=open shadowrootclonable>
        I get cloned!
      </template>
    </div>
  </template>

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 7, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#attr-template-shadowrootdelegatesfocus
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 10, 2024
See the discussion here:

whatwg/html#10107 (comment)

The existing/old behavior was that all declarative shadow roots
have their `clonable` bit set to true, so they automatically get
cloned by `cloneNode()`. The new consensus is that this old behavior
was likely web-incompatible because clones will just start getting
shadow roots included. Plus it wasn't very developer-desirable. The
new behavior adds a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This is a slight behavior change from the existing *shipped*
behavior, in that before the `clonable` concept was introduced,
*any* declarative shadow root within a `<template>` would be
automatically cloned. Now, that behavior is opt in via `clonable`.
So:

pre-`clonable`:
  <template>
    <div>
      <template shadowrootmode=open>
        I get cloned!
      </template>
    </div>
  </template>
  <div>
    <template shadowrootmode=open>
      I do NOT get cloned!
    </template>
  </div>

post-`clonable`, pre-this-CL:
  <template>
    <div>
      <template shadowrootmode=open>
        I get cloned!
      </template>
    </div>
  </template>
  <div>
    <template shadowrootmode=open>
      I ALSO get cloned!
    </template>
  </div>

new as of this CL:
  <template>
    <div>
      <template shadowrootmode=open>
        I do NOT get cloned!
      </template>
    </div>
    <div>
      <template shadowrootmode=open shadowrootclonable>
        I DO get cloned!
      </template>
    </div>
  </template>
  <div>
    <template shadowrootmode=open>
      I do NOT get cloned!
    </template>
  </div>

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 10, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#attr-template-shadowrootdelegatesfocus
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 10, 2024
See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 10, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#attr-template-shadowrootdelegatesfocus
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 10, 2024
See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 10, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#attr-template-shadowrootdelegatesfocus
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 10, 2024
See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258910}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 10, 2024
See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258910}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 10, 2024
See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258910}
@annevk annevk merged commit 531e50d into whatwg:main Feb 13, 2024
2 checks passed
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 13, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 13, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 14, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 15, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5261148
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1260805}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 15, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5261148
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1260805}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 15, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5261148
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1260805}
webkit-commit-queue pushed a commit to annevk/WebKit that referenced this pull request Feb 15, 2024
https://bugs.webkit.org/show_bug.cgi?id=269361

Reviewed by Ryosuke Niwa.

This makes the following changes:

- Adds the new shadowrootclonable attribute to opt into a declarative
  shadow root being clonable.
- As a result, declarative shadow roots are no longer clonable by
  default. Web developers will have to explicitly opt in.
- When attachShadow() is called on a shadow host with an existing
  declarative tree, throw if mode is a mismatch.
- In attachShadow() throw first for mode being set to "user-agent" as
  this is to be caught by the binding layer in theory.
- And finally, only attach a declarative shadow root successfully for
  the first template element.

New tests are upstreamed here:
web-platform-tests/wpt#44568

Specification changes are here (not all have landed yet as various nits
are still being addressed, but all have agreement):

- whatwg/html#10117
- whatwg/html#10069
- whatwg/dom#1246

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/scroll-timeline-name-shadow.html:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/view-timeline-name-shadow.html:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-attachment-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-basic-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats-2-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats-2.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/declarative-shadow-dom-repeats.html:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/declarative/w3c-import.log:
* LayoutTests/imported/w3c/web-platform-tests/shadow-dom/shadow-root-clonable-expected.txt:
* Source/WebCore/dom/Element.cpp:
* Source/WebCore/dom/Element.h:
* Source/WebCore/html/HTMLAttributeNames.in:
* Source/WebCore/html/HTMLTemplateElement.cpp:
(WebCore::HTMLTemplateElement::attachAsDeclarativeShadowRootIfNeeded):
* Source/WebCore/html/parser/HTMLConstructionSite.cpp:
(WebCore::HTMLConstructionSite::insertHTMLTemplateElement):

Canonical link: https://commits.webkit.org/274727@main
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 15, 2024
…re opt-in, a=testonly

Automatic update from web-platform-tests
Change the behavior of clonable to be more opt-in

See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258910}

--

wpt-commits: 33d11f1db34802fda00e64ddeb0b7ef040cf65be
wpt-pr: 44369
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Feb 16, 2024
…re opt-in, a=testonly

Automatic update from web-platform-tests
Change the behavior of clonable to be more opt-in

See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaronchromium.org>
Auto-Submit: Mason Freed <masonfchromium.org>
Commit-Queue: Mason Freed <masonfchromium.org>
Cr-Commit-Position: refs/heads/main{#1258910}

--

wpt-commits: 33d11f1db34802fda00e64ddeb0b7ef040cf65be
wpt-pr: 44369

UltraBlame original commit: 7d4902dfa32c013568c445861c332c29e4c1134f
mbrodesser-Igalia pushed a commit to mbrodesser-Igalia/wpt that referenced this pull request Feb 19, 2024
See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258910}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 19, 2024
…ootClonable reflection, a=testonly

Automatic update from web-platform-tests
Add shadowRootDelegatesFocus and shadowRootClonable reflection

The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5261148
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1260805}

--

wpt-commits: 07a8d151094595d22d2ddf9fc1bda30f6dfa2f1a
wpt-pr: 44386
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Feb 20, 2024
…re opt-in, a=testonly

Automatic update from web-platform-tests
Change the behavior of clonable to be more opt-in

See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaronchromium.org>
Auto-Submit: Mason Freed <masonfchromium.org>
Commit-Queue: Mason Freed <masonfchromium.org>
Cr-Commit-Position: refs/heads/main{#1258910}

--

wpt-commits: 33d11f1db34802fda00e64ddeb0b7ef040cf65be
wpt-pr: 44369

UltraBlame original commit: 7d4902dfa32c013568c445861c332c29e4c1134f
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Feb 20, 2024
…ootClonable reflection, a=testonly

Automatic update from web-platform-tests
Add shadowRootDelegatesFocus and shadowRootClonable reflection

The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5261148
Commit-Queue: Mason Freed <masonfchromium.org>
Reviewed-by: David Baron <dbaronchromium.org>
Reviewed-by: Chris Harrelson <chrishtrchromium.org>
Cr-Commit-Position: refs/heads/main{#1260805}

--

wpt-commits: 07a8d151094595d22d2ddf9fc1bda30f6dfa2f1a
wpt-pr: 44386

UltraBlame original commit: 40ce9b4014176fa6e3ce40fbbbdad17d6339f4d1
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Feb 20, 2024
…re opt-in, a=testonly

Automatic update from web-platform-tests
Change the behavior of clonable to be more opt-in

See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258910}

--

wpt-commits: 33d11f1db34802fda00e64ddeb0b7ef040cf65be
wpt-pr: 44369
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Feb 20, 2024
…ootClonable reflection, a=testonly

Automatic update from web-platform-tests
Add shadowRootDelegatesFocus and shadowRootClonable reflection

The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5261148
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1260805}

--

wpt-commits: 07a8d151094595d22d2ddf9fc1bda30f6dfa2f1a
wpt-pr: 44386
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 23, 2024
See the discussion here:

whatwg/html#10107 (comment)

The existing *shipped* behavior (i.e. before the `clonable` concept
was introduced) was that any declarative shadow root *within a `<template>`* would be automatically cloned, but no others.

The semi-new behavior is the `clonable` bit concept, in which all
declarative shadow roots have their `clonable` bit set to true, so
they automatically get cloned by `cloneNode()`. That's regardless of
whether they are inside or outside a template.

The new consensus is that the "semi-new" clonable behavior is likely
web-incompatible, because clones will just start getting shadow roots
included. Plus it wasn't very developer-desirable. The new consensus
is therefore to add a `shadowrootclonable` attribute for declarative shadow dom that allows a shadow root to opt-in to this behavior, but
the default for all shadow roots will be `clonable=false`.

This CL implements the new consensus behind the ShadowRootClonable
flag. If the flag is false, the "shipped" behavior will be emulated
via setting `clonable` in an equivalent way.

See these three spec PRs:
  whatwg/dom#1246
  whatwg/html#10069
  whatwg/html#10117

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260748
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258910}
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this pull request Feb 23, 2024
The `shadowRootDelegatesFocus` IDL is standardized [1], and was just
never added to Chromium. This CL lands this addition behind a kill
switch, but should be very safe to add.

The `shadowRootClonable` IDL is being added as part of [2], which
has support and should land soon.

[1] https://html.spec.whatwg.org/multipage/scripting.html#htmltemplateelement
[2] whatwg/html#10117

Bug: 1510466
Change-Id: I708ec8d562fa4d0fc59c73a071dbb191e1bd276e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5261148
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1260805}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements normative change topic: shadow Relates to shadow trees (as defined in DOM)
Development

Successfully merging this pull request may close these issues.

3 participants