Closed Bug 2024601 Opened 1 month ago Closed 1 month ago

Resolve `attr(...)` references within container style queries

Categories

(Core :: CSS Parsing and Computation, enhancement)

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
151 Branch
Tracking Flags:
Tracking Status
firefox151 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

(Blocks 3 open bugs)

Details

Attachments

(6 files, 2 obsolete files)

Bug 2024601 - patch 1 - Parsing support for substitution functions as style query expression values. r=#style
48 bytes, text/x-phabricator-request
Details | Review
Bug 2024601 - patch 2 - Add evaluation tests involving substitution functions. r=#style
48 bytes, text/x-phabricator-request
Details | Review
Bug 2024601 - patch 3 - Evaluate substitution functions in style query expression values. r=#style
48 bytes, text/x-phabricator-request
Details | Review
Bug 2024601 - patch 4 - Remove the special-case for simple var() references (without default value) in feature expression values. r=#style
48 bytes, text/x-phabricator-request
Details | Review
Bug 2024601 - patch 5 - Provide an AttributeTracker when resolving style query expression values. r=#style
48 bytes, text/x-phabricator-request
Details | Review
Bug 2024601 - patch 6 - Also support env(), and add a couple tests for it. r=#style
48 bytes, text/x-phabricator-request
Details | Review

One of the testcases in at-container-style-parsing.html fails because it uses CSS attr(...) within a style range query, and we fail to parse & resolve that.

Presumably we need to hook up a suitable AttributeTracker around here and/or here.

Posting this as WIP for now; it doesn't actually work. Need to dig further to
understand what's missing/broken here.

Attachment #9554838 - Attachment is obsolete: true

This resolves our remaining at-container-style-parsing failure.

Also added a few examples with attr() and var(--foo, default) to query-evaluation-style,
otherwise we'd be able to parse without actually evaluating, and yet pass the tests.

(Confirmed that the added evaluation tests also pass in Chrome.)

Assignee: nobody → jfkthame
Status: NEW → ASSIGNED

This is sufficient to make the existing parsing test pass, but the functions don't
actually work at evaluation time. The following patch will add some query evaluation
tests to expose this.

Attachment #9557384 - Attachment is obsolete: true

The existing WPTs check that these forms can be parsed, but not that they actually work
at matching time. So this adds a few examples to the query-evaluation test. They won't
pass in Firefox until support for these functions is hooked up.

(Confirmed that these work as expected in Chrome.)

This makes the examples with var(--foo, default) pass, as well as those with attr()
that depend on the attribute lookup failing (so they use the default); actual attr
lookups will require an AttributeTracker in the substitute() call.

Now that we have more general substitution-function support, the special-case for
var() with a simple dashed-ident (and no default) isn't really necessary. In theory
it would be slightly more efficient, but seems unlikely to be much used as query
expressions also support bare custom-property names (without any var() wrapper),
so authors have no reason to write the more verbose form.

Blocks: 2028142
Pushed by jkew@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/76ede91756ae https://hg.mozilla.org/integration/autoland/rev/08ff9cfc1df9 patch 1 - Parsing support for substitution functions as style query expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/a7d6b5c0ce1b https://hg.mozilla.org/integration/autoland/rev/a7d46dfc0a93 patch 2 - Add evaluation tests involving substitution functions. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/d4728d9065d8 https://hg.mozilla.org/integration/autoland/rev/9f4648cbc99d patch 3 - Evaluate substitution functions in style query expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/9ed150d33b1d https://hg.mozilla.org/integration/autoland/rev/d870f0d9a43f patch 4 - Remove the special-case for simple var() references (without default value) in feature expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/85e1ee18826b https://hg.mozilla.org/integration/autoland/rev/c7042592e5df patch 5 - Provide an AttributeTracker when resolving style query expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/c6c4fb5f1404 https://hg.mozilla.org/integration/autoland/rev/d0ff0e53d7b0 patch 6 - Also support env(), and add a couple tests for it. r=firefox-style-system-reviewers,dshin
Pushed by agoloman@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/58ee6807ff33 https://hg.mozilla.org/integration/autoland/rev/d07c12bf9c4e Revert "Bug 2024601 - patch 6 - Also support env(), and add a couple tests for it. r=firefox-style-system-reviewers,dshin" for causing wr failures @at-supports-namespace-001.html.

Backed out for causing wr failures @at-supports-namespace-001.html.

Flags: needinfo?(jfkthame)

Hmm, so that was actually a pre-existing issue, newly exposed by enabling css attr() for the css-conditional directory. I've moved the pref to enable it only for the container-queries subdir for now.

Flags: needinfo?(jfkthame)
Pushed by jkew@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/c901771e872d https://hg.mozilla.org/integration/autoland/rev/d2766246e689 patch 1 - Parsing support for substitution functions as style query expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/c3158e57d984 https://hg.mozilla.org/integration/autoland/rev/0f2146056519 patch 2 - Add evaluation tests involving substitution functions. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/3adb0b61a49d https://hg.mozilla.org/integration/autoland/rev/f656c1027152 patch 3 - Evaluate substitution functions in style query expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/7c264e231d01 https://hg.mozilla.org/integration/autoland/rev/1e5683bc5a65 patch 4 - Remove the special-case for simple var() references (without default value) in feature expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/b1ff059de9b3 https://hg.mozilla.org/integration/autoland/rev/51e2345bfed0 patch 5 - Provide an AttributeTracker when resolving style query expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/87de35f53311 https://hg.mozilla.org/integration/autoland/rev/edb8021fcb1e patch 6 - Also support env(), and add a couple tests for it. r=firefox-style-system-reviewers,dshin
Pushed by sstanca@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/42e52ce108ca https://hg.mozilla.org/integration/autoland/rev/d86b6b771690 Revert "Bug 2024601 - patch 6 - Also support env(), and add a couple tests for it. r=firefox-style-system-reviewers,dshin" for causing wpt failures in attr-container-style-query.html.

Reverted this because it was causing wpt failures in attr-container-style-query.html.

Flags: needinfo?(jfkthame)
  • Failure line: TEST-UNEXPECTED-PASS | /css/css-values/attr-container-style-query.html | style query should implement to true - expected FAIL

Also (more seriously) shows a bunch of TEST-UNEXPECTED-FAIL on the /css/css-values/attr-cycle.html testcase.

Flags: needinfo?(jfkthame)
Pushed by jkew@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/a17f99d73967 https://hg.mozilla.org/integration/autoland/rev/a2f5abb58176 patch 1 - Parsing support for substitution functions as style query expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/edca4e6c52ff https://hg.mozilla.org/integration/autoland/rev/eeee1edec2ba patch 2 - Add evaluation tests involving substitution functions. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/65fd1a0db700 https://hg.mozilla.org/integration/autoland/rev/f1861dfa7998 patch 3 - Evaluate substitution functions in style query expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/6e0fdc94c34a https://hg.mozilla.org/integration/autoland/rev/4d10545f321f patch 4 - Remove the special-case for simple var() references (without default value) in feature expression values. r=firefox-style-system-reviewers,emilio https://github.com/mozilla-firefox/firefox/commit/3b02d1292b4f https://hg.mozilla.org/integration/autoland/rev/c88e7fb1b91d patch 5 - Provide an AttributeTracker when resolving style query expression values. r=firefox-style-system-reviewers,emilio,dshin https://github.com/mozilla-firefox/firefox/commit/82b73cac260b https://hg.mozilla.org/integration/autoland/rev/26595abfd68c patch 6 - Also support env(), and add a couple tests for it. r=firefox-style-system-reviewers,dshin
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/58953 for changes under testing/web-platform/tests
QA Whiteboard: [qa-triage-done-c152/b151]

Upstream PR was closed without merging

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: