Skip to content

Add test for position outside string in findWordBoundary #30306

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
Open
fantasai wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add test for position outside string in findWordBoundary #30306

fantasai wants to merge 1 commit into from

Conversation

@fantasai
Copy link
Contributor

@fantasai fantasai commented Jun 29, 2024 •
edited by webkit-early-warning-system
Loading

3a4912d

Add test for position outside string in findWordBoundary
https://bugs.webkit.org/show_bug.cgi?id=276002
rdar://130331432

Reviewed by NOBODY (OOPS!).

Checks the output to make sure it is in-range, even if the input is nonsense.

* Tools/TestWebKitAPI/Tests/WebCore/TextBoundaries.cpp:
(TestWebKitAPI::TEST(TextBoundariesTest, FindWordBoundaryEmpty)):
(TestWebKitAPI::TEST(TextBoundariesTest, FindWordBoundaryPositionTooBig)):
(TestWebKitAPI::TEST(TextBoundariesTest, FindWordBoundaryPositionTooSmall)):

3a4912d

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-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
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@fantasai fantasai self-assigned this Jun 29, 2024
@fantasai fantasai added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Jun 29, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 29, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 41b06a4)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 wincairo
  🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-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
  🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@fantasai fantasai requested a review from ddkilzer June 29, 2024 19:49
@fantasai fantasai force-pushed the findWordBoundaryTest branch from 41b06a4 to 8f6af5a Compare June 29, 2024 19:51
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 29, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 8f6af5a)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-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
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

ddkilzer
ddkilzer approved these changes Jun 29, 2024
View reviewed changes
Copy link
Contributor

@ddkilzer ddkilzer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

r=me with adding an underflow (“TooSmall”) test and considering adding expected results for the AppKit platform (macOS).

Thanks!

Tools/TestWebKitAPI/Tests/WebCore/TextBoundaries.cpp Outdated
Copy link
Contributor

@ddkilzer ddkilzer Jun 29, 2024

Choose a reason for hiding this comment

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

Can we provide expected results for the USE(APPKIT) code path, or do they vary on each run?

Copy link
Contributor Author

@fantasai fantasai Jun 30, 2024

Choose a reason for hiding this comment

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

I'm not entirely sure, but the other implementations seem to use different libary or API calls so I don't think we can guarantee the output here.

https://searchfox.org/wubkat/source/Source/WebCore/platform/text/mac/TextBoundaries.mm#181
https://searchfox.org/wubkat/source/Source/WebCore/platform/text/TextBoundaries.cpp#93

Tools/TestWebKitAPI/Tests/WebCore/TextBoundaries.cpp Outdated
Copy link
Contributor

@ddkilzer ddkilzer Jun 29, 2024

Choose a reason for hiding this comment

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

Let’s add an underflow (TooSmall) test as well.

@fantasai fantasai changed the title Add test for position > length in findWordBoundary Add missing return statement Jun 30, 2024
@fantasai fantasai force-pushed the findWordBoundaryTest branch from 8f6af5a to f18660f Compare June 30, 2024 05:01
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2024 •
edited
Loading

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

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2 ✅ 🧪 wincairo-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
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@fantasai fantasai changed the title Add missing return statement Add Jun 30, 2024
@fantasai fantasai force-pushed the findWordBoundaryTest branch from f18660f to 74fc8f5 Compare June 30, 2024 05:18
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 74fc8f5)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-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
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@fantasai fantasai changed the title Add Add test for position outside string in findWordBoundary Jul 1, 2024
@fantasai fantasai force-pushed the findWordBoundaryTest branch from 74fc8f5 to 4e43c90 Compare July 1, 2024 19:58
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 1, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 4e43c90)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2 ✅ 🧪 wincairo-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
✅ 🛠 tv
✅ 🛠 tv-sim
  🛠 watch
✅ 🛠 watch-sim

@fantasai
Add test for position outside string in findWordBoundary
3a4912d 
https://bugs.webkit.org/show_bug.cgi?id=276002
rdar://130331432

Reviewed by NOBODY (OOPS!).

Checks the output to make sure it is in-range, even if the input is nonsense.

* Tools/TestWebKitAPI/Tests/WebCore/TextBoundaries.cpp:
(TestWebKitAPI::TEST(TextBoundariesTest, FindWordBoundaryEmpty)):
(TestWebKitAPI::TEST(TextBoundariesTest, FindWordBoundaryPositionTooBig)):
(TestWebKitAPI::TEST(TextBoundariesTest, FindWordBoundaryPositionTooSmall)):
@fantasai fantasai force-pushed the findWordBoundaryTest branch from 4e43c90 to 3a4912d Compare July 1, 2024 22:34
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 1, 2024 •
edited
Loading

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

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-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
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@nt1m
Copy link
Member

nt1m commented Aug 14, 2025

Can this land?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ddkilzer ddkilzer ddkilzer approved these changes

Assignees

@fantasai fantasai

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.

4 participants

@fantasai @webkit-early-warning-system @nt1m @ddkilzer