Skip to content

CString.isEmpty() for String.utf8() returns true when String has a single character #54762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
webkit-commit-queue merged 1 commit into from
Dec 3, 2025
Merged

CString.isEmpty() for String.utf8() returns true when String has a single character #54762

webkit-commit-queue merged 1 commit into from
Dec 3, 2025

Conversation

@calvaris
Copy link
Contributor

@calvaris calvaris commented Dec 3, 2025 •
edited by webkit-early-warning-system
Loading

1c43f6f

CString.isEmpty() for String.utf8() returns true when String has a single character
https://bugs.webkit.org/show_bug.cgi?id=303428

Reviewed by Darin Adler.

isEmpty was accounting for a null terminator in the CStringBuffer length but it is not like that.

Test: Tools/TestWebKitAPI/Tests/WTF/CString.cpp
* Source/WTF/wtf/text/CString.h:
* Tools/TestWebKitAPI/Tests/WTF/CString.cpp:
(TEST(WTF, CStringEmptyRegularConstructor)):
(TEST(WTF, CStringOnyByte)):

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

98fcf4d

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

@calvaris calvaris self-assigned this Dec 3, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Dec 3, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 7e99966)

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

@calvaris calvaris added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Dec 3, 2025
csaavedra
csaavedra reviewed Dec 3, 2025
View reviewed changes
Tools/TestWebKitAPI/Tests/WTF/CString.cpp Outdated
ASSERT_STREQ(referenceString, stringWithLength.data());
}

TEST(WTF, CStringOnyByte)
Copy link
Member

@csaavedra csaavedra Dec 3, 2025

Choose a reason for hiding this comment

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

That a typo?

Copy link
Member

@darinadler darinadler Dec 3, 2025

Choose a reason for hiding this comment

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

Good catch.

Suggested change
TEST(WTF, CStringOnyByte)
TEST(WTF, CStringOneByte)

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Dec 3, 2025 •
edited
Loading

EWS run on current version of this PR (hash 98fcf4d)

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

@calvaris calvaris added the merge-queue Applied to send a pull request to merge-queue label Dec 3, 2025
@calvaris
CString.isEmpty() for String.utf8() returns true when String has a si…
1c43f6f 
…ngle character

https://bugs.webkit.org/show_bug.cgi?id=303428

Reviewed by Darin Adler.

isEmpty was accounting for a null terminator in the CStringBuffer length but it is not like that.

Test: Tools/TestWebKitAPI/Tests/WTF/CString.cpp
* Source/WTF/wtf/text/CString.h:
* Tools/TestWebKitAPI/Tests/WTF/CString.cpp:
(TEST(WTF, CStringEmptyRegularConstructor)):
(TEST(WTF, CStringOnyByte)):

Canonical link: https://commits.webkit.org/303840@main
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Dec 3, 2025

Committed 303840@main (1c43f6f): https://commits.webkit.org/303840@main

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

@webkit-commit-queue webkit-commit-queue merged commit 1c43f6f into WebKit:main Dec 3, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 3, 2025
@calvaris calvaris deleted the eng/303428 branch December 4, 2025 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@csaavedra csaavedra csaavedra left review comments

@darinadler darinadler darinadler approved these changes

@achristensen07 achristensen07 Awaiting requested review from achristensen07

Assignees

@calvaris calvaris

Labels

New Bugs Unclassified bugs are placed in this component until the correct component can be determined.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@calvaris @webkit-early-warning-system @webkit-commit-queue @csaavedra @darinadler