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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Jonathan Kew [:jfkthame]]

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.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: WIP: Bug 2024601 - [WIP] Pass an AttributeTracker to custom_properties::substitute when resolving query condition.

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

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 2024601 - Support CSS attr() references and var() with fallback in container style query expressions. r=#style

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.)

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Bug 2024601 - patch 1 - Parsing support for substitution functions as style query expression values. r=#style

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.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Bug 2024601 - patch 2 - Add evaluation tests involving substitution functions. r=#style

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.)

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Bug 2024601 - patch 3 - Evaluate substitution functions in style query expression values. r=#style

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.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 2024601 - patch 4 - Remove the special-case for simple var() references (without default value) in feature expression values. r=#style

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.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Bug 2024601 - patch 5 - Provide an AttributeTracker when resolving style query expression values. r=#style

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: Bug 2024601 - patch 6 - Also support env(), and add a couple tests for it. r=#style

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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.

Comment 11

[agoloman]

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

• Backout link
• Push with failures
• Failure Log

Comment 12

[Jonathan Kew [: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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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.

Comment 15

[Serban Stanca [:SerbanS]]

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

• Revert link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-PASS | /css/css-values/attr-container-style-query.html | style query should implement to true - expected FAIL

Comment 16

[Jonathan Kew [: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.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
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

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/58953 for changes under testing/web-platform/tests

Comment 19

[amarc]

https://hg.mozilla.org/mozilla-central/rev/a2f5abb58176
https://hg.mozilla.org/mozilla-central/rev/eeee1edec2ba
https://hg.mozilla.org/mozilla-central/rev/f1861dfa7998
https://hg.mozilla.org/mozilla-central/rev/4d10545f321f
https://hg.mozilla.org/mozilla-central/rev/c88e7fb1b91d
https://hg.mozilla.org/mozilla-central/rev/26595abfd68c

Comment 20

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging