Skip to content

Implement CSS attr() substitution with type() syntax, number, and unit types#60660

Closed
CGQAQ wants to merge 1 commit into
WebKit:mainfrom
CGQAQ:eng/imported-w3c-web-platform-tests-css-css-values-attr-color-invalid-cast-html-fails
Closed

Implement CSS attr() substitution with type() syntax, number, and unit types#60660
CGQAQ wants to merge 1 commit into
WebKit:mainfrom
CGQAQ:eng/imported-w3c-web-platform-tests-css-css-values-attr-color-invalid-cast-html-fails

Conversation

@CGQAQ
Copy link
Copy Markdown
Member

@CGQAQ CGQAQ commented Mar 15, 2026

d47bc4a

Implement CSS attr() substitution with type() syntax, number, and unit types
https://bugs.webkit.org/show_bug.cgi?id=203322
https://bugs.webkit.org/show_bug.cgi?id=203323
https://bugs.webkit.org/show_bug.cgi?id=203324
https://bugs.webkit.org/show_bug.cgi?id=203326
https://bugs.webkit.org/show_bug.cgi?id=203327
https://bugs.webkit.org/show_bug.cgi?id=203328
https://bugs.webkit.org/show_bug.cgi?id=203329

Reviewed by NOBODY (OOPS!).

Extend the CSS variable reference substitution pipeline to handle
attr() as a substitution function per CSS Values 5 specification.
This allows attr() with type(<syntax>), raw-string, number, and
<attr-unit> to work in any CSS property, not just the content property.

When an attr() is encountered during parsing, it is detected as a
variable reference via classifyBlock() and stored as a
CSSVariableReferenceValue. During style resolution, resolveAttrReference()
reads the element attribute, validates it against the specified type,
and substitutes the resolved tokens (or the fallback value).

Key behaviors implemented:
- type(<syntax>): validates attribute value against CSS syntax
- raw-string / omitted type: substitutes as CSS string token
- number: parses attribute as a number token
- <attr-unit> (px, em, %, etc.): parses as number then converts to dimension
- Fallback defaults per spec (empty string when type omitted, guaranteed-invalid otherwise)
- Cycle detection for nested attr() references

FIXME: Substitution of var()/attr()/env() within attribute values
(spec step 6) is not yet implemented to avoid crashes from recursive
custom property resolution.
FIXME: attr()-taint security is not yet implemented.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-content/parsing/content-invalid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-content/parsing/content-invalid.html: update it the latest WPT one
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-IACVT-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-all-types-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-container-style-query-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-cycle-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-invalidation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-length-specified-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-null-namespace-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-pseudo-elem-invalidation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-revert-rule-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-security-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/if-conditionals-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/if-cycle-expected.txt:
* Source/WebCore/css/CSSVariableReferenceValue.cpp:
(WebCore::parseAttrTypeSyntax):
(WebCore::isValidAttrUnit):
(WebCore::parseAttrValueAsNumber):
(WebCore::CSSVariableReferenceValue::resolveAttrReference const):
(WebCore::CSSVariableReferenceValue::resolveTokenRange const):
* Source/WebCore/css/CSSVariableReferenceValue.h:
* Source/WebCore/css/parser/CSSVariableParser.cpp:
(WebCore::classifyBlock):
(WebCore::isValidAttrUnit):
(WebCore::isValidAttrReference):

d47bc4a

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug ✅ 🛠 gtk3-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 ios-safer-cpp ❌ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress 🛠 playstation
✅ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@CGQAQ CGQAQ self-assigned this Mar 15, 2026
@CGQAQ CGQAQ added the CSS Cascading Style Sheets implementation label Mar 15, 2026
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 15, 2026
@CGQAQ CGQAQ marked this pull request as draft March 15, 2026 17:42
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

iOS Safer C++ Build #4538 (0d6fa52)

❌ Found 1 failing file with 2 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

macOS Safer C++ Build #86182 (0d6fa52)

❌ Found 1 failing file with 2 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@CGQAQ CGQAQ removed the merging-blocked Applied to prevent a change from being merged label Mar 15, 2026
@CGQAQ CGQAQ force-pushed the eng/imported-w3c-web-platform-tests-css-css-values-attr-color-invalid-cast-html-fails branch from 0d6fa52 to def5a0a Compare March 15, 2026 18:25
…t types

https://bugs.webkit.org/show_bug.cgi?id=203322
https://bugs.webkit.org/show_bug.cgi?id=203323
https://bugs.webkit.org/show_bug.cgi?id=203324
https://bugs.webkit.org/show_bug.cgi?id=203326
https://bugs.webkit.org/show_bug.cgi?id=203327
https://bugs.webkit.org/show_bug.cgi?id=203328
https://bugs.webkit.org/show_bug.cgi?id=203329

Reviewed by NOBODY (OOPS!).

Extend the CSS variable reference substitution pipeline to handle
attr() as a substitution function per CSS Values 5 specification.
This allows attr() with type(<syntax>), raw-string, number, and
<attr-unit> to work in any CSS property, not just the content property.

When an attr() is encountered during parsing, it is detected as a
variable reference via classifyBlock() and stored as a
CSSVariableReferenceValue. During style resolution, resolveAttrReference()
reads the element attribute, validates it against the specified type,
and substitutes the resolved tokens (or the fallback value).

Key behaviors implemented:
- type(<syntax>): validates attribute value against CSS syntax
- raw-string / omitted type: substitutes as CSS string token
- number: parses attribute as a number token
- <attr-unit> (px, em, %, etc.): parses as number then converts to dimension
- Fallback defaults per spec (empty string when type omitted, guaranteed-invalid otherwise)
- Cycle detection for nested attr() references

FIXME: Substitution of var()/attr()/env() within attribute values
(spec step 6) is not yet implemented to avoid crashes from recursive
custom property resolution.
FIXME: attr()-taint security is not yet implemented.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-content/parsing/content-invalid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-content/parsing/content-invalid.html: update it the latest WPT one
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-IACVT-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-all-types-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-container-style-query-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-cycle-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-invalidation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-length-specified-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-null-namespace-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-pseudo-elem-invalidation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-revert-rule-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/attr-security-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/if-conditionals-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/if-cycle-expected.txt:
* Source/WebCore/css/CSSVariableReferenceValue.cpp:
(WebCore::parseAttrTypeSyntax):
(WebCore::isValidAttrUnit):
(WebCore::parseAttrValueAsNumber):
(WebCore::CSSVariableReferenceValue::resolveAttrReference const):
(WebCore::CSSVariableReferenceValue::resolveTokenRange const):
* Source/WebCore/css/CSSVariableReferenceValue.h:
* Source/WebCore/css/parser/CSSVariableParser.cpp:
(WebCore::classifyBlock):
(WebCore::isValidAttrUnit):
(WebCore::isValidAttrReference):
@CGQAQ CGQAQ force-pushed the eng/imported-w3c-web-platform-tests-css-css-values-attr-color-invalid-cast-html-fails branch from def5a0a to d47bc4a Compare March 15, 2026 18:48
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 15, 2026
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

macOS Safer C++ Build #86189 (def5a0a)

❌ Found 1 failing file with 2 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

iOS Safer C++ Build #4545 (def5a0a)

❌ Found 1 failing file with 2 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@weinig
Copy link
Copy Markdown
Contributor

weinig commented Mar 15, 2026

This needs to be behind a new preference flag.

@weinig
Copy link
Copy Markdown
Contributor

weinig commented Mar 15, 2026

Can you also add a bit more (either here in the PR comments or in the commit message) about the overall plan for this feature? e.g. how does it interact with the existing attr() support that is specific to the content property? How will the equivalent functionality that Style::BuilderState::registerContentAttribute() (and related) currently provide, including how invalidation will work?

It's not a huge problem for this to be in classes called CSSVariableParser/CSSVariableReferenceValue. This is an existing problem, as they don't just support var(), but with each additional feature we add, it gets less ideal.

@CGQAQ
Copy link
Copy Markdown
Member Author

CGQAQ commented Mar 16, 2026

Can you also add a bit more (either here in the PR comments or in the commit message) about the overall plan for this feature?

I'm just poking around to see what's happening. But I guess I need to split the PR when things are sorted out.

e.g. how does it interact with the existing attr() support that is specific to the content property?

If I understand it correctly, the new, modern behavior should eventually completely override the old one?

@CGQAQ CGQAQ closed this Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CSS Cascading Style Sheets implementation merging-blocked Applied to prevent a change from being merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants