Make iOS Find UI reveal matches in scrollable elements
Created attachment 324816 [details] Patch
For some test cases, see attachment 308089 [details] and attachment 312006 [details]
Comment on attachment 324816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324816&action=review > Source/WebKit/ChangeLog:3 > + Make iOS Find UI reveal matches in scrollable elements Can a test be made for this change?
(In reply to Alexey Proskuryakov from comment #3) > Comment on attachment 324816 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=324816&action=review > > > Source/WebKit/ChangeLog:3 > > + Make iOS Find UI reveal matches in scrollable elements > > Can a test be made for this change? It seems that old changes in this part of the code do not have tests... I can test manually, but I'm not sure how to do it in the test runner. Does anyone know how to simulate opening the Find UI and searching some text? Also, note that window.find does not follow the same code path (bug 163911 comment 18) so it does not make sense to use that API to test this change.
Created attachment 324985 [details] Patch (tentative test)
Comment on attachment 324816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324816&action=review >>> Source/WebKit/ChangeLog:3 >>> + Make iOS Find UI reveal matches in scrollable elements >> >> Can a test be made for this change? > > It seems that old changes in this part of the code do not have tests... I can test manually, but I'm not sure how to do it in the test runner. Does anyone know how to simulate opening the Find UI and searching some text? > > Also, note that window.find does not follow the same code path (bug 163911 comment 18) so it does not make sense to use that API to test this change. I tried again today. It seems that the test runner webview does not have the same UI as Safari. Trying to send Cmd+F with eventSender fails (see attachment 324985 [details]). In Safari, we can also use the "Share" menu to execute "Find in Page" but again that UI is not visible in the test runner webview. Finally, I also tried the command document.execCommand("FindString", true, "match") but again, it follows a different code path and won't allow to test that change (see bug 163911 comment 19).
(In reply to Frédéric Wang (:fredw) from comment #6) > Comment on attachment 324816 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=324816&action=review > > >>> Source/WebKit/ChangeLog:3 > >>> + Make iOS Find UI reveal matches in scrollable elements > >> > >> Can a test be made for this change? > > > > It seems that old changes in this part of the code do not have tests... I can test manually, but I'm not sure how to do it in the test runner. Does anyone know how to simulate opening the Find UI and searching some text? > > > > Also, note that window.find does not follow the same code path (bug 163911 comment 18) so it does not make sense to use that API to test this change. > > I tried again today. It seems that the test runner webview does not have the > same UI as Safari. Trying to send Cmd+F with eventSender fails (see > attachment 324985 [details]). In Safari, we can also use the "Share" menu to > execute "Find in Page" but again that UI is not visible in the test runner > webview. Finally, I also tried the command > document.execCommand("FindString", true, "match") but again, it follows a > different code path and won't allow to test that change (see bug 163911 > comment 19). Like smfr said in the other bug, you probably want a UIScriptController function that invokes WKWebView's _findString:options:maxCount:
Created attachment 325159 [details] Patch
(In reply to Tim Horton from comment #7) > Like smfr said in the other bug, you probably want a UIScriptController > function that invokes WKWebView's _findString:options:maxCount: Thanks Tim. I uploaded a new patch with a test to check the effect of the change.
Comment on attachment 325159 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325159&action=review > Source/WebKit/WebProcess/WebPage/FindController.cpp:254 > +#if PLATFORM(IOS) Can we hide this in FindControllerIOS instead? Maybe in didFindString?
Created attachment 325603 [details] Patch
Comment on attachment 325159 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325159&action=review >> Source/WebKit/WebProcess/WebPage/FindController.cpp:254 >> +#if PLATFORM(IOS) > > Can we hide this in FindControllerIOS instead? Maybe in didFindString? Right, thanks. Done in the latest patch.
Committed r224284: <https://trac.webkit.org/changeset/224284>
<rdar://problem/35568141>
Created attachment 353082 [details] Patch (try to implement findString in DumpRenderTree) Simon had suggested to test scrolling on WK1. Just for the record, I'm attaching an attempt to implement UIScriptController::findString for DumpRenderTree's UIScriptControllerIOS. Unfortunately, WebKitLegacy's WebFindOptions does not have the ShowFindIndicator option required by find-text-in-overflow-node.html and WebView does not even expose findString (only the more limited searchFor function). So I believe we can not run this test on WK1.