Skip to content

Limit length of @counter-style padding#60542

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
bnham:eng/Limit-length-of-counter-style-padding
Mar 13, 2026
Merged

Limit length of @counter-style padding#60542
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
bnham:eng/Limit-length-of-counter-style-padding

Conversation

@bnham
Copy link
Copy Markdown
Contributor

@bnham bnham commented Mar 13, 2026

1e7932e

Limit length of @counter-style padding
https://bugs.webkit.org/show_bug.cgi?id=309875
rdar://172044856

Reviewed by Antti Koivisto.

We should limit the length of padding in a @counter-style rule. CSS Counter Styles Level 3
explicitly allows this, and Chrome and Firefox already apply a length limit of 120 and 150
respectively. This patch applies a limit of 150 to match Firefox.

Test: fast/css/counters/counter-style-pad-length-limit.html
* LayoutTests/fast/css/counters/counter-style-pad-length-limit-expected.txt: Added.
* LayoutTests/fast/css/counters/counter-style-pad-length-limit.html: Added.
* Source/WebCore/css/CSSCounterStyle.cpp:
(WebCore::CSSCounterStyle::text):
(WebCore::CSSCounterStyle::applyPadSymbols const):
* Source/WebCore/css/CSSCounterStyle.h:

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

933ac20

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

@bnham bnham self-assigned this Mar 13, 2026
@bnham bnham added the CSS Cascading Style Sheets implementation label Mar 13, 2026
@bnham bnham requested a review from anttijk March 13, 2026 06:37
@bnham bnham force-pushed the eng/Limit-length-of-counter-style-padding branch from 0d94c8c to 933ac20 Compare March 13, 2026 06:52
Comment on lines +459 to +460
result.append(text);
text = result.toString();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use a refactoring where we pass around StringBuilder& instead of a String&

Copy link
Copy Markdown
Contributor Author

@bnham bnham Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I would presume that almost all callers to this function early exit out at the first line since counter style padding seems like it's relatively rare. The initial representation of the counter passed in to this function is a String (and changing that seems like probably not a good idea but that's just based on my local scanning of the code). So at first glance it seems like probably a good idea to leave the function signature alone.

I mostly changed the String repeat building here since I'm already mucking with this function, and it seems easy to reason about, and it seems like all the other similar functions in this file are already using a StringBuilder to build up counter-related strings.

@bnham bnham added the merge-queue Applied to send a pull request to merge-queue label Mar 13, 2026
https://bugs.webkit.org/show_bug.cgi?id=309875
rdar://172044856

Reviewed by Antti Koivisto.

We should limit the length of padding in a @counter-style rule. CSS Counter Styles Level 3
explicitly allows this, and Chrome and Firefox already apply a length limit of 120 and 150
respectively. This patch applies a limit of 150 to match Firefox.

Test: fast/css/counters/counter-style-pad-length-limit.html
* LayoutTests/fast/css/counters/counter-style-pad-length-limit-expected.txt: Added.
* LayoutTests/fast/css/counters/counter-style-pad-length-limit.html: Added.
* Source/WebCore/css/CSSCounterStyle.cpp:
(WebCore::CSSCounterStyle::text):
(WebCore::CSSCounterStyle::applyPadSymbols const):
* Source/WebCore/css/CSSCounterStyle.h:

Canonical link: https://commits.webkit.org/309228@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Limit-length-of-counter-style-padding branch from 933ac20 to 1e7932e Compare March 13, 2026 20:21
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 309228@main (1e7932e): https://commits.webkit.org/309228@main

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

@webkit-commit-queue webkit-commit-queue merged commit 1e7932e into WebKit:main Mar 13, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 13, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants