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

Should there be an opt-out for declarative shadow roots having clonable=true? #10107

Closed
mfreed7 opened this issue Jan 29, 2024 · 27 comments
Closed
Labels
topic: shadow Relates to shadow trees (as defined in DOM)

Comments

@mfreed7
Copy link
Contributor

mfreed7 commented Jan 29, 2024

What is the issue with the HTML Standard?

The spec for declarative shadow DOM says:

  • Attach a shadow root with declarative shadow host element, declarative shadow mode, true, declarative shadow delegates focus, and "named".

The third true argument there corresponds to the clonable flag, meaning all declarative shadow roots are clonable. It seems like there should be a way to opt out of that behavior somehow, e.g.

<template shadowrootmode=open notclonable>

or something like that. Otherwise, every declarative shadow root becomes clonable. There is some concern from Chromium that shipping the general clonable behavior might be a web-incompatible change, for sites that use SSR/DSD and assume those shadow roots aren't cloned. @justinfagnani raised this concern for Lit users, for example.

@annevk @emilio

@justinfagnani
Copy link

justinfagnani commented Jan 29, 2024

Sorry, I somehow missed the clonable aspect of DSD entirely.

I think it's very much not true that just because a shadow root was created via DSD that it would be valid to clone it later.

For Lit, there are at least two major problems that make cloning problematic:

  1. For client side rendering we don't emit the necessary extra DOM structure in order to rehydrate. We don't emit template IDs to be able to re-associate DOM to the template that rendered it, and we don't emit marker comments for attribute bindings. The code to do this isn't even part of the client-side packages.
  2. A shadow root could easily go through state-driven changes before being cloned where that state is not available to the clone. Without something like cloneCallback() we don't even have a spot to clone that state. This will make the cloned shadow root useless, as we'd have to clear it out and re-render to be in sync with the clone's actual state.

So I'm pretty confident that right now many, if not most, cloned SSR'ed Lit components would fail, and our only fix would be to detect this case (we would need to differentiate from an actual DSD somehow) and clear the shadow root, completely defeating the purpose of cloning in the first place.

I would rather see cloneable be an opt-in.

@emilio
Copy link
Contributor

emilio commented Jan 29, 2024

cloneable being opt in makes more sense for whatwg/dom#1246 and co, actually. Otherwise any new non-declarative shadow DOM that you attach is likely to throw...

@mfreed7
Copy link
Contributor Author

mfreed7 commented Jan 29, 2024

cloneable being opt in makes more sense for whatwg/dom#1246 and co, actually. Otherwise any new non-declarative shadow DOM that you attach is likely to throw...

Gah, that's a very good point. That would completely break the use case for attachShadow() emptying and returning the declarative shadow root - to avoid breaking old components that don't know about SSR.

I'm starting to wonder about the rest of the behavior in whatwg/dom#1246 actually. I wonder if the behavior should be that it throws if the call to attachShadow() explicitly passes an argument that disagrees with the existing shadow root, but doesn't throw if that parameter is not passed. E.g.

<div><template shadowrootmode=open></template></div>
<script>
  div.attachShadow({mode: open, clonable: false}); // Throws - clonable mismatch
  div.attachShadow({mode: open}); // Don't throw because `clonable` not passed explicitly
</script>

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 29, 2024
This parameter was added ([1]) after the initial check code was written
([3]). Note that [2] also added `clonable` but there's an issue [4]
about checking that in a similar way.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/5239221
[2] https://chromium-review.googlesource.com/c/chromium/src/+/5239277
[3] https://chromium-review.googlesource.com/c/chromium/src/+/5191750
[4] whatwg/html#10107

Bug: 1517959
Change-Id: I689c6a6f84fb7ce298cf187076ae228fef6cc348
@annevk annevk added the topic: shadow Relates to shadow trees (as defined in DOM) label Jan 30, 2024
@annevk
Copy link
Member

annevk commented Jan 30, 2024

Why did we make declarative shadow roots clonable by default? whatwg/dom#831 and whatwg/dom#1137 are not very clear on this.

Also, hasn't this already shipped as part of declarative shadow trees?

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 30, 2024
This parameter was added ([1]) after the initial check code was written
([3]). Note that [2] also added `clonable` but there's an issue [4]
about checking that in a similar way.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/5239221
[2] https://chromium-review.googlesource.com/c/chromium/src/+/5239277
[3] https://chromium-review.googlesource.com/c/chromium/src/+/5191750
[4] whatwg/html#10107

Bug: 1517959
Change-Id: I689c6a6f84fb7ce298cf187076ae228fef6cc348
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 30, 2024
This parameter was added ([1]) after the initial check code was written
([3]). Note that [2] also added `clonable` but there's an issue [4]
about checking that in a similar way.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/5239221
[2] https://chromium-review.googlesource.com/c/chromium/src/+/5239277
[3] https://chromium-review.googlesource.com/c/chromium/src/+/5191750
[4] whatwg/html#10107

Bug: 1517959
Change-Id: I689c6a6f84fb7ce298cf187076ae228fef6cc348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5242281
Commit-Queue: David Baron <[email protected]>
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1253972}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 30, 2024
This parameter was added ([1]) after the initial check code was written
([3]). Note that [2] also added `clonable` but there's an issue [4]
about checking that in a similar way.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/5239221
[2] https://chromium-review.googlesource.com/c/chromium/src/+/5239277
[3] https://chromium-review.googlesource.com/c/chromium/src/+/5191750
[4] whatwg/html#10107

Bug: 1517959
Change-Id: I689c6a6f84fb7ce298cf187076ae228fef6cc348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5242281
Commit-Queue: David Baron <[email protected]>
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1253972}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 30, 2024
This parameter was added ([1]) after the initial check code was written
([3]). Note that [2] also added `clonable` but there's an issue [4]
about checking that in a similar way.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/5239221
[2] https://chromium-review.googlesource.com/c/chromium/src/+/5239277
[3] https://chromium-review.googlesource.com/c/chromium/src/+/5191750
[4] whatwg/html#10107

Bug: 1517959
Change-Id: I689c6a6f84fb7ce298cf187076ae228fef6cc348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5242281
Commit-Queue: David Baron <[email protected]>
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1253972}
@mfreed7
Copy link
Contributor Author

mfreed7 commented Jan 30, 2024

Why did we make declarative shadow roots clonable by default? whatwg/dom#831 and whatwg/dom#1137 are not very clear on this.

Chrome's originally shipped behavior (and initial spec PR) only cloned declarative shadow roots inside template elements. It modified only the template cloning steps. As part of the review, that was changed to the currently spec'd behavior with the clonable flag. At the time that change was made, I didn't realize the implications of that for DSD roots outside templates. (I should have.)

Having said all of that, the use case was, and still is, to allow declarative shadow DOM to be used in a <template> that gets stamped out and still contains the shadow roots. Cloning general shadow roots is perhaps not a real world use case, I'm not sure. I'd be happy if we went back to the original proposal of just cloning shadow roots within templates, but I'm open to suggestions. Perhaps clonable can be made to be true by default only for DSD within a <template>? That would at least be web compatible for sure.

Also, hasn't this already shipped as part of declarative shadow trees?

The behavior that already shipped (in 2020 originally, and no change was made to this part in the re-shipment with shadowrootmode in 2023) is to clone shadow roots found in <template>s.

@annevk
Copy link
Member

annevk commented Feb 1, 2024

I think it would be better if:

  1. By default they do not clone, if that is indeed the desired behavior.
  2. Adding a shadowrootclonable attribute makes them clonable.

So if you use them inside template you'd have to add that attribute.

@mfreed7
Copy link
Contributor Author

mfreed7 commented Feb 1, 2024

I think it would be better if:

  1. By default they do not clone, if that is indeed the desired behavior.
  2. Adding a shadowrootclonable attribute makes them clonable.

So if you use them inside template you'd have to add that attribute.

I think I like this proposal. It's a bit of a change from the existing shipped behavior, so there might be compat issues. But it feels like there's a very straightforward migration path for that breakage, to simply add shadowrootclonable. And it feels a lot less risky than the currently-spec'd behavior.

I'm willing to give this a try to see what the compat implications are, if there's consensus enough to get that change landed in the spec first?

@emilio
Copy link
Contributor

emilio commented Feb 2, 2024

That sounds reasonable to me, and makes clonable consistent with all other attributes, and with itself in the scripting and non-scripting case.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue 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>

Bug: 1510466
Change-Id: Ice7c7579094eb08b882c4bb44f93045f23b8f222
mfreed7 added a commit to mfreed7/html that referenced this issue Feb 2, 2024
…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
@mfreed7
Copy link
Contributor Author

mfreed7 commented Feb 2, 2024

Alright, I wrote that up in #10117. It'll have a merge conflict with #10069 once that lands, but I can fix it up after that. The DOM PR (whatwg/dom#1246) doesn't need changes to accommodate this, since it already checks clonable and that now seems ok since clonable is opt-in everywhere.

If we're good with this, let's land the spec changes, and I'll try shipping this new behavior in Chrome to make sure it's web compatible.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue 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 issue 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
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 2, 2024
…achShadow, a=testonly

Automatic update from web-platform-tests
Check for matching `serializable` in attachShadow

This parameter was added ([1]) after the initial check code was written
([3]). Note that [2] also added `clonable` but there's an issue [4]
about checking that in a similar way.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/5239221
[2] https://chromium-review.googlesource.com/c/chromium/src/+/5239277
[3] https://chromium-review.googlesource.com/c/chromium/src/+/5191750
[4] whatwg/html#10107

Bug: 1517959
Change-Id: I689c6a6f84fb7ce298cf187076ae228fef6cc348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5242281
Commit-Queue: David Baron <[email protected]>
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1253972}

--

wpt-commits: 9d88a7f08bafe30d73d49d0b9d026d4a4603916e
wpt-pr: 44246
@justinfagnani
Copy link

We chatted about this on the Lit team yesterday, and I think we'd like a bit more discussion on the intention and use cases for this feature if possible.

I'm somewhat sympathetic to the idea that trees with declarative shadow roots should be clonable with the idea that one would expect a deep clone to produce the same tree and that it functions the same. Was that the intention behind this feature?

The problems arise after the yet-to-be-cloned shadow root is modified. One way around those problems would be making the shadow root non-clonable during hydration, so it could still be cloned when we knew it had all the correct structure to be hydrated but not clone when it may be out-of-sync.

So, could clonable be mutable?

@mfreed7
Copy link
Contributor Author

mfreed7 commented Feb 2, 2024

So, could clonable be mutable?

I was previously reluctant to do this, but now, having implemented the rest of this feature, I think that'd be trivial. The clonable flag is just checked at clone time, so there's no harm (and nothing to be done) when flipping it between false/true.

@annevk @emilio any objections to that small change? If not I'll incorporate it.

ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Feb 2, 2024
…achShadow, a=testonly

Automatic update from web-platform-tests
Check for matching `serializable` in attachShadow

This parameter was added ([1]) after the initial check code was written
([3]). Note that [2] also added `clonable` but there's an issue [4]
about checking that in a similar way.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/5239221
[2] https://chromium-review.googlesource.com/c/chromium/src/+/5239277
[3] https://chromium-review.googlesource.com/c/chromium/src/+/5191750
[4] whatwg/html#10107

Bug: 1517959
Change-Id: I689c6a6f84fb7ce298cf187076ae228fef6cc348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5242281
Commit-Queue: David Baron <[email protected]>
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1253972}

--

wpt-commits: 9d88a7f08bafe30d73d49d0b9d026d4a4603916e
wpt-pr: 44246
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue 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
@emilio
Copy link
Contributor

emilio commented Feb 2, 2024

Any objections to that small change? If not I'll incorporate it.

I think that same simplicity argument could apply as well to other members (delegatesFocus at least).

I'm not sure I object but then throwing for those in attachShadow seems silly (could be better to "override only if present" or so). So maybe worth doing / discussing in a separate issue.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue 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
aarongable pushed a commit to chromium/chromium that referenced this issue 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1258910}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1258910}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1258910}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 10, 2024
Per the discussion:

whatwg/html#10107

This CL makes `clonable` mutable. It also makes `attachShadow()`
mutate that bit rather than throwing an exception if there's a
mismatching declarative shadow root present. It also makes the
`serializable` bit behave the same way.

Bug: 1510466
Change-Id: Ia3f4b6c18677c7f3d3acd52ba74ea0eb4a31849a
@annevk annevk closed this as completed in 531e50d Feb 13, 2024
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 13, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 13, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 14, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 15, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1258910}

--

wpt-commits: 33d11f1db34802fda00e64ddeb0b7ef040cf65be
wpt-pr: 44369
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 15, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue 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
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 16, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 17, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062,325598615
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 17, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062,325598615
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 17, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062,325598615
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5283935
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Di Zhang <[email protected]>
Reviewed-by: Di Zhang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262014}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 17, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062,325598615
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5283935
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Di Zhang <[email protected]>
Reviewed-by: Di Zhang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262014}
mbrodesser-Igalia pushed a commit to mbrodesser-Igalia/wpt that referenced this issue 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1258910}
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue 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
jamienicol pushed a commit to jamienicol/gecko that referenced this issue 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1258910}

--

wpt-commits: 33d11f1db34802fda00e64ddeb0b7ef040cf65be
wpt-pr: 44369
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 23, 2024
…adow, a=testonly

Automatic update from web-platform-tests
Only check for matching mode in attachShadow

Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062,325598615
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5283935
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Di Zhang <[email protected]>
Reviewed-by: Di Zhang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262014}

--

wpt-commits: a092215fbf94a99eee2137327b452db80f52a7ad
wpt-pr: 44562
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Feb 23, 2024
This parameter was added ([1]) after the initial check code was written
([3]). Note that [2] also added `clonable` but there's an issue [4]
about checking that in a similar way.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/5239221
[2] https://chromium-review.googlesource.com/c/chromium/src/+/5239277
[3] https://chromium-review.googlesource.com/c/chromium/src/+/5191750
[4] whatwg/html#10107

Bug: 1517959
Change-Id: I689c6a6f84fb7ce298cf187076ae228fef6cc348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5242281
Commit-Queue: David Baron <[email protected]>
Reviewed-by: David Baron <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1253972}
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1258910}
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Feb 23, 2024
Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062,325598615
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5283935
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Di Zhang <[email protected]>
Reviewed-by: Di Zhang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262014}
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Feb 24, 2024
…adow, a=testonly

Automatic update from web-platform-tests
Only check for matching mode in attachShadow

Per the new-new consensus, attachShadow will only verify that
the existing declarative shadow root's `mode` matches the newly
requested `mode`:

whatwg/html#10107 (comment)

Bug: 41483062,325598615
Change-Id: Ie3bac4ec297c0b85c40b45495e9c823dd47cb49e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5283935
Commit-Queue: Mason Freed <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Di Zhang <[email protected]>
Reviewed-by: Di Zhang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1262014}

--

wpt-commits: a092215fbf94a99eee2137327b452db80f52a7ad
wpt-pr: 44562
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: shadow Relates to shadow trees (as defined in DOM)
Development

No branches or pull requests

5 participants
@justinfagnani @emilio @annevk @mfreed7 and others