Simplify NameValidation by using contains and containsOnly by annevk · Pull Request #65869 · WebKit/WebKit

annevk · 2026-05-28T07:23:24Z

`dd8ad14`

Simplify NameValidation by using contains and containsOnly
https://bugs.webkit.org/show_bug.cgi?id=315737
rdar://178698891

Reviewed by Ryosuke Niwa and Darin Adler.

This does result in the first code unit being checked twice for the
checks that have special rules for the first code unit, but using
substring seemed more expensive. And using contains and containsOnly
should be more efficient as there's less is8Bit branching.

Tested neutral on Speedometer.

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

f37ad0e

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
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim	❌ 🧪 mac-site-isolation
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2026-06-01T12:17:56Z

EWS run on previous version of this PR (hash 2afe329)

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

rniwa

We should probably perf A/B test this change with at least 20 iterations on macOS and iOS.

darinadler

I agree that we should check performance on this. It seems likely that having a separate branch for 8-bit will make this faster, so it’s possible the new version is faster!

darinadler · 2026-06-01T18:14:40Z

-        return true;
-    }
+    if (isASCIIAlpha(firstCharacter))
+        return !name.contains(isInvalidElementNameCharacterAfterAlphaStart);


This re-checks the first character. The old code skipped that first character. That might not have been an important optimization, but it was intentional. We could keep that optimization by using name.substring(1) instead of name.

I see now that you said “using substring seemed more expensive”. What does “seemed” mean here? Is that a guess about performance or about code size?

It’s a guess. Cost of a StringView vs checking an additional code unit.

darinadler · 2026-06-01T18:15:00Z

-        return true;
-    }
+    if (firstCharacter == ':' || firstCharacter == '_' || firstCharacter >= 0x80)
+        return name.containsOnly<isValidElementNameContinuationCharacter>();


This re-checks the first character. The old code skipped that first character. That might not have been an important optimization, but it was intentional. We could keep that optimization by using name.substring(1) instead of name.

I see now that you said “using substring seemed more expensive”. What does “seemed” mean here? Is that a guess about performance or about code size?

darinadler · 2026-06-01T18:15:23Z

-            return false;
-    }
-    return true;
+    return name.containsOnly<isValidASCIIXMLNamePart>();


This re-checks the first character. The old code skipped that first character. That might not have been an important optimization, but it was intentional. We could keep that optimization by using name.substring(1) instead of name.

I see now that you said “using substring seemed more expensive”. What does “seemed” mean here? Is that a guess about performance or about code size?

webkit-early-warning-system · 2026-06-05T09:18:37Z

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

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	✅ 🧪 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
✅ 🛠 🧪 unsafe-merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim	❌ 🧪 mac-site-isolation
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-ews-buildbot · 2026-06-05T13:39:59Z

Failed api-ios checks. Please resolve failures and re-apply safe-merge-queue label.

Rejecting #65869 from merge queue.

webkit-ews-buildbot · 2026-06-05T13:40:03Z

Safe-Merge-Queue: Build #94573.

https://bugs.webkit.org/show_bug.cgi?id=315737 rdar://178698891 Reviewed by Ryosuke Niwa and Darin Adler. This does result in the first code unit being checked twice for the checks that have special rules for the first code unit, but using substring seemed more expensive. And using contains and containsOnly should be more efficient as there's less is8Bit branching. Tested neutral on Speedometer. Canonical link: https://commits.webkit.org/314632@main

webkit-commit-queue · 2026-06-05T13:50:23Z

Committed 314632@main (dd8ad14): https://commits.webkit.org/314632@main

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

annevk self-assigned this May 28, 2026

annevk added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label May 28, 2026

This comment was marked as outdated.

Sign in to view

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 28, 2026

This comment was marked as outdated.

Sign in to view

annevk removed the merging-blocked Applied to prevent a change from being merged label May 31, 2026

annevk force-pushed the eng/Simplify-NameValidation-by-using-contains-and-containsOnly branch from 18921ee to df99d30 Compare May 31, 2026 05:00

This comment was marked as outdated.

Sign in to view

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 31, 2026

annevk removed the merging-blocked Applied to prevent a change from being merged label May 31, 2026

annevk force-pushed the eng/Simplify-NameValidation-by-using-contains-and-containsOnly branch from df99d30 to f3165a8 Compare May 31, 2026 06:26

This comment was marked as outdated.

Sign in to view

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 31, 2026

annevk removed the merging-blocked Applied to prevent a change from being merged label Jun 1, 2026

annevk force-pushed the eng/Simplify-NameValidation-by-using-contains-and-containsOnly branch from f3165a8 to 2afe329 Compare June 1, 2026 12:16

annevk marked this pull request as ready for review June 1, 2026 12:47

annevk requested review from cdumez and rniwa as code owners June 1, 2026 12:47

annevk requested a review from darinadler June 1, 2026 12:47

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 1, 2026

rniwa approved these changes Jun 1, 2026

View reviewed changes

darinadler approved these changes Jun 1, 2026

View reviewed changes

annevk removed the merging-blocked Applied to prevent a change from being merged label Jun 5, 2026

annevk force-pushed the eng/Simplify-NameValidation-by-using-contains-and-containsOnly branch from 2afe329 to f37ad0e Compare June 5, 2026 09:18

annevk added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jun 5, 2026

webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Jun 5, 2026

annevk added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Jun 5, 2026

webkit-commit-queue force-pushed the eng/Simplify-NameValidation-by-using-contains-and-containsOnly branch from f37ad0e to dd8ad14 Compare June 5, 2026 13:50

webkit-commit-queue merged commit dd8ad14 into WebKit:main Jun 5, 2026

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 5, 2026

annevk deleted the eng/Simplify-NameValidation-by-using-contains-and-containsOnly branch June 5, 2026 13:51

Conversation

annevk commented May 28, 2026 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

dd8ad14

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

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

Uh oh!

Uh oh!

rniwa left a comment

Choose a reason for hiding this comment

Uh oh!

darinadler left a comment

Choose a reason for hiding this comment

Uh oh!

darinadler Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

darinadler Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darinadler Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

webkit-ews-buildbot commented Jun 5, 2026

Uh oh!

webkit-ews-buildbot commented Jun 5, 2026

Uh oh!

webkit-commit-queue commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

annevk commented May 28, 2026 •

edited by webkit-early-warning-system

Loading

`dd8ad14`

webkit-early-warning-system commented Jun 1, 2026 •

edited

Loading

darinadler Jun 1, 2026 •

edited

Loading

darinadler Jun 1, 2026 •

edited

Loading

darinadler Jun 1, 2026 •

edited

Loading

webkit-early-warning-system commented Jun 5, 2026 •

edited

Loading