[css-anchor-position-1] Implement parsing support for anchor-center value by Scony · Pull Request #35517 · WebKit/WebKit

Scony · 2024-10-21T13:25:34Z

`c0f2f40`

[css-anchor-position-1] Implement parsing support for anchor-center value
https://bugs.webkit.org/show_bug.cgi?id=281846

Reviewed by Tim Nguyen.

This change adds parsing support for `anchor-center` CSS value as specified in:
https://drafts.csswg.org/css-anchor-position-1/#anchor-center

* LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/anchor-center-001-expected.txt:
* Source/WebCore/css/CSSPrimitiveValueMappings.h:
* Source/WebCore/css/CSSProperties.json:
* Source/WebCore/css/CSSValueKeywords.in:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Align.cpp:
(WebCore::CSSPropertyParserHelpers::isSelfPositionKeyword):
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::alignmentOffset):
* Source/WebCore/rendering/RenderGrid.cpp:
(WebCore::RenderGrid::columnAxisPositionForGridItem const):
(WebCore::RenderGrid::rowAxisPositionForGridItem const):
* Source/WebCore/rendering/style/RenderStyleConstants.cpp:
(WebCore::operator<<):
* Source/WebCore/rendering/style/RenderStyleConstants.h:

Canonical link: https://commits.webkit.org/285641@main

3df0c5d

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	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	⏳ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-10-21T13:25:41Z

EWS run on previous version of this PR (hash f8e1f72)

Details

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	❌ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	❌ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

Source/WebCore/css/CSSProperties.json

webkit-early-warning-system · 2024-10-22T11:50:16Z

EWS run on previous version of this PR (hash 228fb71)

Details

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	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	⏳ 🛠 vision	⏳ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	⏳ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	⏳ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	⏳ 🛠 tv	✅ 🛠 mac-safer-cpp
	⏳ 🛠 tv-sim
	⏳ 🛠 watch
	⏳ 🛠 watch-sim

webkit-early-warning-system · 2024-10-23T09:09:21Z

EWS run on previous version of this PR (hash 47ddd35)

Details

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	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	🛠 vision	~~🧪 mac-AS-debug-wk2~~	🧪 gtk-wk2
	~~🛠 vision-sim~~	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	⏳ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

nt1m

LGTM, feel free to land this

tuankiet65 · 2024-10-23T17:51:15Z

LGTM. I'd prefer that the TODOs have links to the bugzilla to implement them, but I suppose you're close to getting them work anyway.

webkit-early-warning-system · 2024-10-23T19:13:47Z

EWS run on current version of this PR (hash 3df0c5d)

Details

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	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	⏳ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

Scony · 2024-10-23T19:14:57Z

LGTM. I'd prefer that the TODOs have links to the bugzilla to implement them, but I suppose you're close to getting them work anyway.

I've added the links as in few days I'm starting holidays, so it will take at least few weeks before I land the followup.

…alue https://bugs.webkit.org/show_bug.cgi?id=281846 Reviewed by Tim Nguyen. This change adds parsing support for `anchor-center` CSS value as specified in: https://drafts.csswg.org/css-anchor-position-1/#anchor-center * LayoutTests/imported/w3c/web-platform-tests/css/css-anchor-position/anchor-center-001-expected.txt: * Source/WebCore/css/CSSPrimitiveValueMappings.h: * Source/WebCore/css/CSSProperties.json: * Source/WebCore/css/CSSValueKeywords.in: * Source/WebCore/css/parser/CSSPropertyParserConsumer+Align.cpp: (WebCore::CSSPropertyParserHelpers::isSelfPositionKeyword): * Source/WebCore/rendering/RenderFlexibleBox.cpp: (WebCore::alignmentOffset): * Source/WebCore/rendering/RenderGrid.cpp: (WebCore::RenderGrid::columnAxisPositionForGridItem const): (WebCore::RenderGrid::rowAxisPositionForGridItem const): * Source/WebCore/rendering/style/RenderStyleConstants.cpp: (WebCore::operator<<): * Source/WebCore/rendering/style/RenderStyleConstants.h: Canonical link: https://commits.webkit.org/285641@main

webkit-commit-queue · 2024-10-24T09:26:51Z

Committed 285641@main (c0f2f40): https://commits.webkit.org/285641@main

Reviewed commits have been landed. Closing PR #35517 and removing active labels.

yisibl · 2024-10-25T04:14:03Z

Does the current parsing support unsafe anchor-center?

Scony · 2024-10-25T06:20:05Z

Does the current parsing support unsafe anchor-center?

No - unless previous implementation supported it - in that case it does as a side effect.

nt1m · 2024-10-25T07:23:02Z

Does the current parsing support unsafe anchor-center?

No - unless previous implementation supported it - in that case it does as a side effect.

It should work with this patch, since anything in isSelfPositionKeyword should work with safe/unsafe.

Though I did notice that the parser-grammar-unused changes in this patch are incorrect. They should be undone, and anchor-center should be added here instead: https://searchfox.org/wubkat/rev/1e937703de6af995ef2cd93867292c9da1d3b86c/Source/WebCore/css/CSSProperties.json#10997

yisibl · 2024-10-25T07:58:06Z

I mention it because this case requires the unsafe anchor-center.
https://x.com/tabatkins/status/1849581984143777956

Scony · 2024-10-25T13:06:30Z

Does the current parsing support unsafe anchor-center?

No - unless previous implementation supported it - in that case it does as a side effect.

It should work with this patch, since anything in isSelfPositionKeyword should work with safe/unsafe.

Though I did notice that the parser-grammar-unused changes in this patch are incorrect. They should be undone, and anchor-center should be added here instead: https://searchfox.org/wubkat/rev/1e937703de6af995ef2cd93867292c9da1d3b86c/Source/WebCore/css/CSSProperties.json#10997

Good point, I'll fix that before adding actual implementation.

As for unsafe anchor-center - I'll take a deeper look at it by the way.

Scony self-assigned this Oct 21, 2024

Scony added the CSS Cascading Style Sheets implementation label Oct 21, 2024

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 21, 2024

nt1m reviewed Oct 21, 2024

View reviewed changes

Source/WebCore/css/CSSProperties.json Outdated Show resolved Hide resolved

Scony removed the merging-blocked Applied to prevent a change from being merged label Oct 22, 2024

Scony force-pushed the 281846-css-anchoring-anchor-center-parsing branch from f8e1f72 to 228fb71 Compare October 22, 2024 11:50

nt1m approved these changes Oct 22, 2024

View reviewed changes

Scony marked this pull request as ready for review October 22, 2024 18:32

Scony force-pushed the 281846-css-anchoring-anchor-center-parsing branch from 228fb71 to 47ddd35 Compare October 23, 2024 09:09

Scony requested a review from anttijk October 23, 2024 09:45

nt1m approved these changes Oct 23, 2024

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 23, 2024

Scony removed the merging-blocked Applied to prevent a change from being merged label Oct 23, 2024

Scony force-pushed the 281846-css-anchoring-anchor-center-parsing branch from 47ddd35 to 3df0c5d Compare October 23, 2024 19:13

Scony added the merge-queue Applied to send a pull request to merge-queue label Oct 24, 2024

webkit-commit-queue force-pushed the 281846-css-anchoring-anchor-center-parsing branch from 3df0c5d to c0f2f40 Compare October 24, 2024 09:26

webkit-commit-queue merged commit c0f2f40 into WebKit:main Oct 24, 2024

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 24, 2024

Conversation

Scony commented Oct 21, 2024 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

c0f2f40

Uh oh!

webkit-early-warning-system commented Oct 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

webkit-early-warning-system commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nt1m left a comment

Choose a reason for hiding this comment

Uh oh!

tuankiet65 commented Oct 23, 2024

Uh oh!

webkit-early-warning-system commented Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Scony commented Oct 23, 2024

Uh oh!

webkit-commit-queue commented Oct 24, 2024

Uh oh!

yisibl commented Oct 25, 2024

Uh oh!

Scony commented Oct 25, 2024

Uh oh!

nt1m commented Oct 25, 2024

Uh oh!

yisibl commented Oct 25, 2024

Uh oh!

Scony commented Oct 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Scony commented Oct 21, 2024 •

edited by webkit-early-warning-system

Loading

`c0f2f40`

webkit-early-warning-system commented Oct 21, 2024 •

edited

Loading

webkit-early-warning-system commented Oct 22, 2024 •

edited

Loading

webkit-early-warning-system commented Oct 23, 2024 •

edited

Loading

webkit-early-warning-system commented Oct 23, 2024 •

edited

Loading