FindController::findString will always either increment or decrement the match index. This is incorrect when the highlighted match is the same as before. For example, say the search string "Hello" highlights some text in the sentence "Hello, world.". Match count is (for example) 10 and match index is 0. All is well. The search is refined to "Hello, world" and the highlighted area is expanded according to the larger match range, but does not move to a different match on the page. The match index increments to 1 despite highlighting the same match as before.
<rdar://problem/55352425>
Created attachment 380265 [details] Patch
Comment on attachment 380265 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380265&action=review > Source/WebCore/ChangeLog:10 > + Add NoIndexChange find option flag No indentation > Source/WebKit/ChangeLog:22 > + Do not change match index if NoIndexChange bit is set. Otherwise, no change in behavior. This is indented wrong > Source/WebKit/Shared/WebFindOptions.h:39 > + FindOptionsNoIndexChange = 1 << 8, It's not a problem in this case, but a bit odd to add to the middle of an enum. > Source/WebKit/WebProcess/WebPage/FindController.cpp:70 > + if (options & FindOptionsNoIndexChange) > + result.add(WebCore::NoIndexChange); You can avoid the WebCore change and the core() change; nothing ever uses this option in WebCore, right?
I bet you can write an API test for this; peek at TestWebKitAPI's WKWebViewFindString.mm (or maybe one of the others).
Created attachment 380341 [details] Patch
Looks like the API test fails
Created attachment 380345 [details] Patch
(In reply to Tim Horton from comment #6) > Looks like the API test fails Added test to old API -- oops. This run should be good.
Created attachment 380349 [details] Patch
Comment on attachment 380349 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380349&action=review > Source/WebKit/Shared/API/c/WKFindOptions.h:44 > + kWKFindOptionsShowHighlight = 1 << 7, > + kWKFindOptionsNoIndexChange = 1 << 8 You shouldn't have to add this > Source/WebKit/Shared/API/c/WKSharedAPICast.h:772 > + if (wkFindOptions & kWKFindOptionsNoIndexChange) > + findOptions |= FindOptionsNoIndexChange; Or this. > Source/WebKit/Shared/WebFindOptions.h:40 > + FindOptionsNoIndexChange = 1 << 8, > + FindOptionsDetermineMatchIndex = 1 << 9, Technically fine in this case because it's internal only but still weird to add things in the middle of the enum. > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewFindString.mm:192 > + [firstWebView _findString:@"hello" options:findOptions maxCount:maxCount]; This is ObjC API so it should always take ObjC API enums (_WKFindOptions and _WKFindOptionsNoIndexChange). What went wrong that you have to use the C API constant?! Looking at the implementation of _findString:options:maxCount: in WKWebView, it uses the toFindOptions() function that's right above it, which goes from _WKFindOptions->WebKit::FindOptions, so I think this is just wrong.
Created attachment 380909 [details] Patch
Comment on attachment 380909 [details] Patch Clearing flags on attachment: 380909 Committed r251111: <https://trac.webkit.org/changeset/251111>
All reviewed patches have been landed. Closing bug.