[CSS Zoom] Fix zoom factor for inherited letter-spacing by vitorroriz · Pull Request #51906 · WebKit/WebKit

vitorroriz · 2025-10-07T02:10:08Z

`a3cf1b7`

[CSS Zoom] Fix zoom factor for inherited letter-spacing
rdar://162085369
https://bugs.webkit.org/show_bug.cgi?id=300278

Reviewed by Antti Koivisto.

We want to store the unzoomed letter-spacing value at RenderStyle
and the zoomeed value at FontCascade.

Test: imported/w3c/web-platform-tests/css/css-text/parsing/letter-spacing-inherited-computed.html
* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/letter-spacing-inherited-computed-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/letter-spacing-inherited-computed.html: Added.
* Source/WebCore/style/StyleBuilderState.cpp:
(WebCore::Style::BuilderState::updateFontForSizeChange):
* Source/WebCore/style/values/text/StyleLetterSpacing.cpp:
(WebCore::Style::CSSValueConversion<LetterSpacing>::operator):
* Source/WebCore/style/values/text/StyleLetterSpacing.h:

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

6104906

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	✅ 🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	~~🧪 win-tests~~	✅ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		✅ 🛠 vision-apple
	~~🧪 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	❌ 🛠 playstation
	✅ 🛠 tv	❌ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-10-07T02:10:15Z

EWS run on previous version of this PR (hash 654223d)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	✅ 🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	❌ 🧪 win-tests	✅ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		❌ 🛠 vision-apple
	✅ 🧪 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	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

nullhook · 2025-10-08T19:07:57Z

+                auto usedZoom = shouldUseEvaluationTimeZoom(state) ? 1.0f : state.style().usedZoom();
+                return usedZoom * textZoomFactor;
            }
            return state.cssToLengthConversionData().zoom();


the else branch here does not seem to respect the shouldUseEvaluationTimeZoom

I'm not sure how to produce an observable test that exercises this branch. I've tried playing with a detached doc but I couldn't observe its layout properties.

looks like the code here is being overly defensive. we can't enter style-resolution without a frame. so, that else branch looks meaningless.

nt1m · 2025-10-08T23:01:45Z

@@ -1,3 +1,4 @@
+<!-- webkit-test-runner [ EvaluationTimeZoomEnabled=true ] -->


no longer needed after the other PRs

webkit-early-warning-system · 2025-10-08T23:14:12Z

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

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	⏳ 🛠 win	✅ 🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	⏳ 🧪 win-tests	✅ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe		⏳ 🛠 vision-apple
	❌ 🧪 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	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

vitorroriz · 2025-10-08T23:43:56Z

zoom/letter-spacing.html will be failing until gets merged #52027

weinig · 2025-10-09T00:27:31Z

Like with the others, this needs tests for the useSVGZoomRulesForLength cases.

weinig · 2025-10-09T00:29:23Z

Would also like to see tests to make sure getComputedValue continues to produce the unzoomed value for the various inheritance cases.

vitorroriz · 2025-10-09T00:48:30Z

Like with the others, this needs tests for the useSVGZoomRulesForLength cases.
@weinig
I've created a test that mimics LayoutTests/imported/w3c/web-platform-tests/css/css-viewport/zoom/letter-spacing.html. for SVG. We scale it wrong (different from other browsers) due to unrelated reasons to this PR, which I believe is related to RenderSVGInlineText::computeNewScaledFontForStyle and RenderSVGInlineText::computeScalingFactorForRenderer and should be fixed apart.

vitorroriz · 2025-10-09T00:52:55Z

Would also like to see tests to make sure getComputedValue continues to produce the unzoomed value for the various inheritance cases.

I can create a new WPT test for that but we do apply the unzoomed value to RenderStyle and the zoomed value to FontCascade

webkit-early-warning-system · 2025-10-09T01:48:05Z

EWS run on previous version of this PR (hash 0c1def0)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	⏳ 🛠 win	🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	⏳ 🧪 win-tests	✅ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	~~🧪 api-wpe~~		✅ 🛠 vision-apple
	✅ 🧪 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~~	❌ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-10-09T02:37:14Z

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

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	~~🛠 wpe~~	⏳ 🛠 win	✅ 🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	⏳ 🧪 win-tests	✅ 🛠 mac-apple
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~		🛠 vision-apple
	~~🧪 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~~	~~🛠 playstation~~
	✅ 🛠 tv	~~🛠 mac-safer-cpp~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-10-09T02:48:23Z

EWS run on previous version of this PR (hash 02356eb)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	💥 🧪 win-tests	✅ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		✅ 🛠 vision-apple
	✅ 🧪 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	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

nullhook

computed tests should be housed under ...css/css-viewport/zoom/parsing/ otherwise lgtm.

webkit-early-warning-system · 2025-10-09T17:46:00Z

EWS run on current version of this PR (hash 6104906)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	✅ 🛠 ios-apple
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	~~🧪 win-tests~~	✅ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		✅ 🛠 vision-apple
	~~🧪 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	❌ 🛠 playstation
	✅ 🛠 tv	❌ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

rdar://162085369 https://bugs.webkit.org/show_bug.cgi?id=300278 Reviewed by Antti Koivisto. We want to store the unzoomed letter-spacing value at RenderStyle and the zoomeed value at FontCascade. Test: imported/w3c/web-platform-tests/css/css-text/parsing/letter-spacing-inherited-computed.html * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/letter-spacing-inherited-computed-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-text/parsing/letter-spacing-inherited-computed.html: Added. * Source/WebCore/style/StyleBuilderState.cpp: (WebCore::Style::BuilderState::updateFontForSizeChange): * Source/WebCore/style/values/text/StyleLetterSpacing.cpp: (WebCore::Style::CSSValueConversion<LetterSpacing>::operator): * Source/WebCore/style/values/text/StyleLetterSpacing.h: Canonical link: https://commits.webkit.org/301269@main

webkit-commit-queue · 2025-10-09T18:53:12Z

Committed 301269@main (a3cf1b7): https://commits.webkit.org/301269@main

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

vitorroriz self-assigned this Oct 7, 2025

vitorroriz added the Text For bugs in text layout and rendering, including international text support. label Oct 7, 2025

vitorroriz requested review from nullhook and weinig October 7, 2025 05:33

vitorroriz marked this pull request as ready for review October 7, 2025 20:43

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

vitorroriz removed the merging-blocked Applied to prevent a change from being merged label Oct 7, 2025

vitorroriz requested review from brentfulgham and mdubet October 8, 2025 19:00

nullhook reviewed Oct 8, 2025

View reviewed changes

nt1m reviewed Oct 8, 2025

View reviewed changes

vitorroriz force-pushed the eng/CSS-Zoom-Fix-zoom-factor-for-inherited-letter-spacing branch from 654223d to b438655 Compare October 8, 2025 23:14

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

vitorroriz removed the merging-blocked Applied to prevent a change from being merged label Oct 9, 2025

vitorroriz force-pushed the eng/CSS-Zoom-Fix-zoom-factor-for-inherited-letter-spacing branch from b438655 to 0c1def0 Compare October 9, 2025 01:47

vitorroriz force-pushed the eng/CSS-Zoom-Fix-zoom-factor-for-inherited-letter-spacing branch from 0c1def0 to eda344f Compare October 9, 2025 02:37

vitorroriz force-pushed the eng/CSS-Zoom-Fix-zoom-factor-for-inherited-letter-spacing branch from eda344f to 02356eb Compare October 9, 2025 02:48

vitorroriz requested a review from anttijk October 9, 2025 17:02

nullhook approved these changes Oct 9, 2025

View reviewed changes

anttijk approved these changes Oct 9, 2025

View reviewed changes

vitorroriz force-pushed the eng/CSS-Zoom-Fix-zoom-factor-for-inherited-letter-spacing branch from 02356eb to 6104906 Compare October 9, 2025 17:32

vitorroriz added the merge-queue Applied to send a pull request to merge-queue label Oct 9, 2025

webkit-commit-queue force-pushed the eng/CSS-Zoom-Fix-zoom-factor-for-inherited-letter-spacing branch from 6104906 to a3cf1b7 Compare October 9, 2025 18:53

webkit-commit-queue merged commit a3cf1b7 into WebKit:main Oct 9, 2025

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

vitorroriz deleted the eng/CSS-Zoom-Fix-zoom-factor-for-inherited-letter-spacing branch November 4, 2025 13:32

		@@ -1,3 +1,4 @@
		<!-- webkit-test-runner [ EvaluationTimeZoomEnabled=true ] -->

Conversation

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

Uh oh!

a3cf1b7

Uh oh!

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

Uh oh!

Uh oh!

nullhook Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

vitorroriz Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

nullhook Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

nt1m Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

vitorroriz commented Oct 8, 2025

Uh oh!

weinig commented Oct 9, 2025

Uh oh!

weinig commented Oct 9, 2025

Uh oh!

vitorroriz commented Oct 9, 2025

Uh oh!

vitorroriz commented Oct 9, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

nullhook left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

webkit-commit-queue commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

vitorroriz commented Oct 7, 2025 •

edited by webkit-early-warning-system

Loading

`a3cf1b7`

webkit-early-warning-system commented Oct 7, 2025 •

edited

Loading

webkit-early-warning-system commented Oct 8, 2025 •

edited

Loading

webkit-early-warning-system commented Oct 9, 2025 •

edited

Loading

webkit-early-warning-system commented Oct 9, 2025 •

edited

Loading

webkit-early-warning-system commented Oct 9, 2025 •

edited

Loading

webkit-early-warning-system commented Oct 9, 2025 •

edited

Loading