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

[css-counter-styles] Clarify how @counter-style name lookup works with shadow DOM #5693

Closed
xiaochengh opened this issue Nov 4, 2020 · 5 comments
Labels
Closed Accepted as Obvious Bugfix Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-counter-styles-3 Current Work

Comments

@xiaochengh
Copy link
Contributor

  1. How should @counter-style rules defined in shadow DOM work. The current spec totally ignores it and says

Defining a @counter-style makes it available to the entire document in which it is included.

The current behavior (available in Firefox only) is:

The spec should probably be revised to match the proposal in #1995


  1. How do extends system and fallback descriptor search for the correct rule with shadow DOM?

This is more complicated. If we have a @counter-style rule in a stylesheet that is reused by multiple shadow DOMs, where does it search for the rule to extend or fallback? Obviously we can't just search within the same stylesheet, while searching the entire document and ignoring scoping also seems wrong.

The proposal in #1995 doesn't work, either, because a @counter-style rule isn't associated with any tree scope when defined. And it may be reused by multiple tree scopes.

Note: The situation is similar to @Property. We currently don't allow @Property in shadow DOMs but it's likely to change (w3c/css-houdini-drafts#939)

@fantasai fantasai added the css-counter-styles-3 Current Work label Nov 18, 2020
@xiaochengh
Copy link
Contributor Author

Here's a proposal based on CSS-SCOPING-1, and compatible with Firefox's behavior that a global @counter-style rule can be access within a shadow tree.

As specified in CSS-SCOPING-1:

  • Each @counter-style rule defines a tree-scoped name
  • Each counter style name reference defines a tree-scoped reference. This includes:
    • system: extends and fallback descriptors
    • list-style-type property
    • counter() and counters() functions in content property

To dereference a tree-scoped reference, we first look for names within the same tree scope; If there's no match, we recursively walk up to the parent tree scope and search until a matching name is found.

Note that the current spec of CSS-SCOPING-1 only searches the same tree scope, which makes @counter-style defined in the main document inaccessible from shadow trees.

@tabatkins
Copy link
Member

Yeah, it should work exactly the same way as for @font-face, aka "not remotely defined right now, but I have a years-old proposal for fixing it that I need someone to sign off on implementing".

pull bot pushed a commit to Alan-love/chromium that referenced this issue Dec 9, 2020
This patch adds a new structure CounterStyleMap to each tree scope.
There is also one map for user rules, and one map to user agent rules.
CounterStyleMap indexes all the counter styles defined in the tree
scopes, and also has logic for:
- Given a tree scope and a name, resolving the tree-scoped reference
  following proposal [1]
- Resolve and update 'extends' and 'fallback' references after active
  style changes

Note that we cannot share CounterStyleMap between tree scopes even if
they have the same rule set. This is because the same rule may have its
'extends' and 'fallback' resolved differently when used in different
tree scopes.

The complicated parts are:
- We must resolve references in parent scope before child scopes, but
  ScopedStyleResolver::AppendActiveStyleSheets() is not called in the
  tree order
- We need to update references when active style changes. Moreover,
  an active style change in one tree scope may affect references in
  the child scopes, and it's hard to keep track which tree scopes
  are affected. So we end up updating all unconditionally
- We need to detect cycles in 'extends'

Therefore, the reference resolution and update is done for all tree
scopes at the end of StyleEngine::UpdateActiveStyleSheets().

[1] w3c/csswg-drafts#5693 (comment)

Bug: 687225
Change-Id: Ib907103438c5c996f6abfc81f4ff22fd932a827a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2574064
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835021}
@tabatkins
Copy link
Member

The current spec in Scoping intentionally doesn't walk up the tree, to better increase encapsulation, but I suppose it makes sense that these refs can "inherit" down into shadows. I suspect it better matches the mishmash of behavior for scoped references browsers currently have, anyway; at least, I think "you can always use the stuff from the top-level doc" is pretty common among them.

I'll go adjust Scoping to match that, and then explicitly ref it here.

@fantasai
Copy link
Collaborator

Agenda+ to review fix with the CSSWG.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-counter-styles] Clarify how @counter-style name lookup works with shadow DOM, and agreed to the following:

  • RESOLVED: Accept the PR linked in the issue defining how counter style scoping works with shadow dom
The full IRC log of that discussion <dael> Topic: [css-counter-styles] Clarify how @counter-style name lookup works with shadow DOM
<dael> github: https://github.com//issues/5693
<dael> TabAtkins: We talked previous week about the small change to name defining constructs to work with scoping. This is a request for approval to edit counter styles so that it works with the scoping machinery. Define counter style in outer page you can reference it within shadow dom. If you define it within the same name it stays in the shadow and overrides
<dael> TabAtkins: If you define it in the outer, inherit into a shadow, and redefine you get what you think you should use. Nothing novel, but spec needed a change to hook into that and I wanted approval b/c it is normative
<dael> astearns: Prop: Accept the PR linked in the issue defining how counter style scoping works with shadow dom
<dael> astearns: Obj?
<dael> RESOLVED: Accept the PR linked in the issue defining how counter style scoping works with shadow dom

@fantasai fantasai closed this as completed Jun 3, 2021
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 1, 2021
Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 13, 2021
Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 13, 2021
Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3000883
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900782}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 13, 2021
Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3000883
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900782}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Jul 13, 2021
Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3000883
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900782}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 18, 2021
… tests to WPT, a=testonly

Automatic update from web-platform-tests
[@counter-style] Move shadow-DOM-related tests to WPT

Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3000883
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900782}

--

wpt-commits: abbafb6d4eb22478494cb98d1f25f4ba5174b0bc
wpt-pr: 29560
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 20, 2021
… tests to WPT, a=testonly

Automatic update from web-platform-tests
[@counter-style] Move shadow-DOM-related tests to WPT

Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3000883
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900782}

--

wpt-commits: abbafb6d4eb22478494cb98d1f25f4ba5174b0bc
wpt-pr: 29560
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jul 20, 2021
… tests to WPT, a=testonly

Automatic update from web-platform-tests
[@counter-style] Move shadow-DOM-related tests to WPT

Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3000883
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900782}

--

wpt-commits: abbafb6d4eb22478494cb98d1f25f4ba5174b0bc
wpt-pr: 29560
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jul 21, 2021
… tests to WPT, a=testonly

Automatic update from web-platform-tests
[@counter-style] Move shadow-DOM-related tests to WPT

Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3000883
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900782}

--

wpt-commits: abbafb6d4eb22478494cb98d1f25f4ba5174b0bc
wpt-pr: 29560
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
This patch adds a new structure CounterStyleMap to each tree scope.
There is also one map for user rules, and one map to user agent rules.
CounterStyleMap indexes all the counter styles defined in the tree
scopes, and also has logic for:
- Given a tree scope and a name, resolving the tree-scoped reference
  following proposal [1]
- Resolve and update 'extends' and 'fallback' references after active
  style changes

Note that we cannot share CounterStyleMap between tree scopes even if
they have the same rule set. This is because the same rule may have its
'extends' and 'fallback' resolved differently when used in different
tree scopes.

The complicated parts are:
- We must resolve references in parent scope before child scopes, but
  ScopedStyleResolver::AppendActiveStyleSheets() is not called in the
  tree order
- We need to update references when active style changes. Moreover,
  an active style change in one tree scope may affect references in
  the child scopes, and it's hard to keep track which tree scopes
  are affected. So we end up updating all unconditionally
- We need to detect cycles in 'extends'

Therefore, the reference resolution and update is done for all tree
scopes at the end of StyleEngine::UpdateActiveStyleSheets().

[1] w3c/csswg-drafts#5693 (comment)

Bug: 687225
Change-Id: Ib907103438c5c996f6abfc81f4ff22fd932a827a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2574064
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835021}
GitOrigin-RevId: 0976b66f60d21b57b45f06b3d2b3a2f71c378656
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
This patch adds some new tests for how @counter-style rules with shadow
DOM. Since the CSSWG spec issue [1] is still open and the spec text
hasn't been modified, the new tests are added into wpt_internal/.

[1] w3c/csswg-drafts#5693

Bug: 687225
Change-Id: Ie801eb969d71feee47ed2d3b3480445cc9cd2527
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2691398
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#853634}
GitOrigin-RevId: 0a02a52febf55f1fe89e8915dc2bfea26a9e3391
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Now that the name scoping behavior has been specified in the way these
tests assert [1], this patch moves them to WPT.

[1] w3c/csswg-drafts#5693

Bug: 1225033
Change-Id: I3970ec8a1ee185ad32fcd1e7c57952d5513e6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3000883
Auto-Submit: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900782}
NOKEYCHECK=True
GitOrigin-RevId: 710c233f104db318e87684e85f53d4a1d2d694e3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted as Obvious Bugfix Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-counter-styles-3 Current Work
Projects
None yet
Development

No branches or pull requests

4 participants