Skip to content
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.

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

REGRESSION(274325@main): Find matches can not be selected after calling findMatchesForString #24982

Conversation

charliewolfe
Copy link
Member

@charliewolfe charliewolfe commented Feb 22, 2024

4671253

REGRESSION(274325@main): Find matches can not be selected after calling `findMatchesForString`
https://bugs.webkit.org/show_bug.cgi?id=269825
rdar://123465751

Reviewed by Aditya Keerthi.

Before 274325@main, `FindController::updateFindUIAfterPageScroll` would clear `m_findMatches` only if it
was called from `FindController::findString`. After 274325@main, all call sites will clear it, so a
client will not be able to set selection after calling `findMatchesForString`. To match the previous
behavior we can check if a frame identifier was passed, since that will only be provided by
`FindController::findString`.

* Source/WebKit/WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::updateFindUIAfterPageScroll):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:
(TEST):

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

eb7a38d

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@charliewolfe charliewolfe self-assigned this Feb 22, 2024
@charliewolfe charliewolfe added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Feb 22, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 23, 2024
@charliewolfe charliewolfe 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 Feb 23, 2024
…ng `findMatchesForString`

https://bugs.webkit.org/show_bug.cgi?id=269825
rdar://123465751

Reviewed by Aditya Keerthi.

Before 274325@main, `FindController::updateFindUIAfterPageScroll` would clear `m_findMatches` only if it
was called from `FindController::findString`. After 274325@main, all call sites will clear it, so a
client will not be able to set selection after calling `findMatchesForString`. To match the previous
behavior we can check if a frame identifier was passed, since that will only be provided by
`FindController::findString`.

* Source/WebKit/WebProcess/WebPage/FindController.cpp:
(WebKit::FindController::updateFindUIAfterPageScroll):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:
(TEST):

Canonical link: https://commits.webkit.org/275219@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION274325main-Find-matches-can-not-be-selected-after-calling-findMatchesForString branch from eb7a38d to 4671253 Compare February 23, 2024 05:34
@webkit-commit-queue
Copy link
Collaborator

Committed 275219@main (4671253): https://commits.webkit.org/275219@main

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

@webkit-commit-queue webkit-commit-queue merged commit 4671253 into WebKit:main Feb 23, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants