2009001 - [css-attr] Fix failing type(...) parsing tests.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Swarup Ukil [:sukil]]

A number of type(...) tests in attr-all-types.html fail like:

• test_valid_attr('--y', 'attr(data-foo type(*))', 'var(--x, 3)', '3');
• test_valid_attr('--y', 'attr(data-foo type(*))', 'attr(data-bar, 11)', '"3"');

These tests expect recursive resolution. I.e. data-foo=attr(data-bar, 3) to be resolved as 3. Right now we resolve it as type(<string>) so data-foo substitutes as "attr(data-bar, 11)" but the test would expect 3.

Comment 1

[David Shin[:dshin]]

Basically, need to support recursive substitution.
Recursive substitution of attr shouldn't be super complicated, but attr -> var substitution could get complicated with registered properties that defer its computation.

Comment 2

[David Shin[:dshin]]

So we need to make correct deferring of attr() substitution for e.g. For --bar: attr(foo) + foo="var(--registered-length)" + --registered-length: 5em; (Depending on attribute, depending on registered property, depending on font-size). Also need to handle circular references e.g. font-size: attr(foo) + foo="var(--registered-length)" + --registered-length: 5em` (Depending on attribute, depending on registered property, depending on a computed value that depends on font size).

Ultimately, this means that we need to do a bit more legwork and try to parse the attribute in question like a declaration value, like we do for fallbacks - These would store references to other properties/attributes to be used during substitution time here for fallbacks

As for when that needs to happen... Well, attributes can be used for prioritary properties, so to be able to detect circular references by then, we need to have references populated by the first substitution time. OTOH we can't handle it exactly like fallbacks, because attributes depend on which element we're styling.

Comment 3

[Swarup Ukil [:sukil]]

Attachment: Bug 2009001 - Support chained references in attr when type() syntax is used. r=emilio

Comment 4

[Swarup Ukil [:sukil]]

Attachment: Bug 2009001 - Fix attr substitution test using invalid width value. r=dshin

The test previously expected width: 3, which passed without
proper attr() substitution. It now compares against width: 3px
using data-baz=3px to test for correct substitution.

Comment 5

[David Shin[:dshin]]

Attachment: Bug 2009001: Inline all AllSubstitutionFunctions and ComputedSubstitutionFunctions functions. r=#style

Comment 6

[David Shin[:dshin]]

Attachment: Bug 2009001: New attr-including maps are copy-on-write. r=#style

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b09e47c25222
https://hg.mozilla.org/integration/autoland/rev/2d1dba54adb6
Support chained references in attr when type() syntax is used. r=firefox-style-system-reviewers,dshin
https://github.com/mozilla-firefox/firefox/commit/d2c68e854c95
https://hg.mozilla.org/integration/autoland/rev/034bbef95b7e
Fix attr substitution test using invalid width value. r=dshin
https://github.com/mozilla-firefox/firefox/commit/38f583eb0f82
https://hg.mozilla.org/integration/autoland/rev/f99176341264
Inline all AllSubstitutionFunctions and ComputedSubstitutionFunctions functions. r=firefox-style-system-reviewers,sukil,emilio

Comment 8

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

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

Comment 9

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/2d1dba54adb6
https://hg.mozilla.org/mozilla-central/rev/034bbef95b7e
https://hg.mozilla.org/mozilla-central/rev/f99176341264