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-cascade] When an import rule fails to load or has an unsatisfied condition, does the layer still count? #6776

Closed
xiaochengh opened this issue Oct 28, 2021 · 4 comments

Comments

@xiaochengh
Copy link
Contributor

Consider @import rules below:

@import url(url-not-found.css) layer(A);
@import url(sheet.css) layer(B) not all;
@import url(sheet.css) layer(C) supports(display: invalid-value);

While these are valid but ineffective imports, do the layer declarations still count (and hence affect the global layer ordering)? The current spec seems unclear about it.

I suppose for conditional imports whose conditions evaluate to false, the layer declarations should not count, because nested @layer rules inside false media/supports rules do not count.

I'm unsure about imports that fail to load, though. We may count the layer for a stable global layer ordering, or do not counter the layer to be consistent with media/supports conditions. Either as a resolution will be fine for me.

@xiaochengh xiaochengh changed the title [css-cascade] When an import rule fails to load or isn't enabled, does the layer still count? [css-cascade] When an import rule fails to load or has an unsatisfied condition, does the layer still count? Oct 28, 2021
@mirisuzanne mirisuzanne added this to To Resolve in Cascade 5 (Layers) Oct 28, 2021
@mirisuzanne
Copy link
Contributor

mirisuzanne commented Oct 28, 2021

The way I understand the spec, we should already have implied answers to all of these, which we can clarify if we need to:

  • layer A is established, but empty (stable layer order)
  • layers B and C are not established since the global queries are false

@tabatkins
Copy link
Member

The precise interleaving of layers and conditions in the import rule isn't specified, so we should be clearer about this, but I agree with @mirisuzanne on the details of how each case should work. The mental model should be that a layer is wrapped around the contents of the import (even if they're empty due to a load failure), and a condition is wrapped around the whole thing:

@import url(...) layer(foo) (condition: bar);

/* equivalent to */

@media (condition: bar) {
 @layer foo {
   ...import contents...
 }
}

@mirisuzanne mirisuzanne moved this from To Resolve to To Draft in Cascade 5 (Layers) Nov 9, 2021
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 10, 2021
As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 10, 2021
As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272669
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940505}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 10, 2021
As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272669
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940505}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Nov 11, 2021
As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272669
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940505}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 15, 2021
…vior on failed/unmatched imports, a=testonly

Automatic update from web-platform-tests
[@layer] Ensure layer establishment behavior on failed/unmatched imports

As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272669
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940505}

--

wpt-commits: c21a4bab57696f57f98fe063fd976e88e6556de4
wpt-pr: 31586
@mirisuzanne
Copy link
Contributor

Agenda+ to get this spec change approved by the group.

@mirisuzanne mirisuzanne moved this from To Draft to To Resolve in Cascade 5 (Layers) Nov 16, 2021
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 16, 2021
…vior on failed/unmatched imports, a=testonly

Automatic update from web-platform-tests
[@layer] Ensure layer establishment behavior on failed/unmatched imports

As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272669
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940505}

--

wpt-commits: c21a4bab57696f57f98fe063fd976e88e6556de4
wpt-pr: 31586
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-cascade] When an import rule fails to load or has an unsatisfied condition, does the layer still count?, and agreed to the following:

  • RESOLVED: Accept proposal
  • RESOLVED: Republish css-cascade-5
The full IRC log of that discussion <fantasai> Topic: [css-cascade] When an import rule fails to load or has an unsatisfied condition, does the layer still count?
<fantasai> github: https://github.com//issues/6776
<fantasai> miriam: A few questions in here
<fantasai> miriam: All related to establishing layers inside of an @import statement.
<fantasai> miriam: One question is what to do if the import fails
<fantasai> miriam: other question is what to do if the conditions of the import don't match
<fantasai> miriam: consensus in the thread was that we should conceptualize this to "layer is wrapped around contents of the import, even if empty due to failure, and condition is wrapped around the whole thing"
<fantasai> miriam: We added language to spec to express it that way
<fantasai> miriam: layer is still added to layer order if import fails
<fantasai> miriam: but not added if the conditions don't match
<fantasai> miriam: that matches the behavior of if you had import inside of layer inside of condition
<fantasai> miriam: wanted to check with WG if this is acceptable
<fantasai> jensimmons: Makes a lot of sense from authoring point of view
<fantasai> +1 from me
<fantasai> RESOLVED: Accept proposal
<fantasai> miriam: This allows us to republish
<fantasai> RESOLVED: Republish css-cascade-5

Gabisampaio pushed a commit to Gabisampaio/wpt that referenced this issue Nov 18, 2021
As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272669
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940505}
@fantasai fantasai closed this as completed Dec 2, 2021
@mirisuzanne mirisuzanne moved this from To Resolve to To Review/Publish in Cascade 5 (Layers) Jan 31, 2022
@mirisuzanne mirisuzanne moved this from To Review/Publish to Complete in Cascade 5 (Layers) Aug 4, 2022
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272669
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940505}
NOKEYCHECK=True
GitOrigin-RevId: e1a0f46a6fa0c0b07677d1afa048d0d9021a083f
ns-rsilva pushed a commit to ns-rsilva/chromium that referenced this issue Apr 25, 2024
As per w3c/csswg-drafts#6776:
1. @import with unmatched media query shouldn't establish layer
2. @import that fails to load should still establish layer

This patch adds test cases for it and also fixes our behavior on 1.

Bug: 1095765
Change-Id: Ia7d2c9921b4b93173546cbd78ab243dac3f92420
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3272669
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940505}

Former-commit-id: e1a0f46a6fa0c0b07677d1afa048d0d9021a083f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

5 participants