Bug 192084

Summary: [iOS] Page not defocused when Find-in-page becomes first responder
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebKit Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, jer.noble, megan_gardner, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar, PlatformOnly
Version: WebKit Local Build   
Hardware: iPhone / iPad   
OS: iOS 12   
See Also: https://bugs.webkit.org/show_bug.cgi?id=190017
https://bugs.webkit.org/show_bug.cgi?id=133098
https://bugs.webkit.org/show_bug.cgi?id=192165
Attachments:
Description Flags
Patch and unit tests
none
Patch and unit tests
thorton: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews125 for ios-simulator-wk2 none

Daniel Bates
Reported 2018-11-28 09:52:28 PST
Splitting off the fix for "focusing" from bug #133098. When WKWebView is resign as a responder, say as a result of the Find-in-page UI becoming first responder, then the page should become defocused. Otherwise, WebKit will become confused and may take actions as if it was focused when it is not. For example, following <https://trac.webkit.org/changeset/236619/> (bug #190017) WebKit will think it can focus text fields whose content matches the Find-in-page search string.
Attachments
Patch and unit tests (16.89 KB, patch)
2018-11-28 10:06 PST, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Patch and unit tests (16.86 KB, patch)
2018-11-28 10:11 PST, Daniel Bates 		thorton: review+
ews-watchlist: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (333.54 KB, application/zip)
2018-11-28 11:18 PST, EWS Watchlist 		no flags
Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Daniel Bates
Comment 1 2018-11-28 09:52:35 PST
<rdar://problem/46146777>
Daniel Bates
Comment 2 2018-11-28 10:06:42 PST
Created attachment 355884 [details] Patch and unit tests
Daniel Bates
Comment 3 2018-11-28 10:11:48 PST
Created attachment 355885 [details] Patch and unit tests
Tim Horton
Comment 4 2018-11-28 10:54:53 PST
Comment on attachment 355885 [details] Patch and unit tests View in context: https://bugs.webkit.org/attachment.cgi?id=355885&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewFindString.h:2 > + * Copyright (C) 2018 Apple Inc. All rights reserved. Why the header? Most of our tests don't have corresponding headers.
EWS Watchlist
Comment 5 2018-11-28 11:18:35 PST
Comment on attachment 355885 [details] Patch and unit tests Attachment 355885 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/10184286 Number of test failures exceeded the failure limit.
EWS Watchlist
Comment 6 2018-11-28 11:18:37 PST
Created attachment 355895 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Daniel Bates
Comment 7 2018-11-28 12:54:20 PST
(In reply to Tim Horton from comment #4) > Comment on attachment 355885 [details] > Patch and unit tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=355885&action=review > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewFindString.h:2 > > + * Copyright (C) 2018 Apple Inc. All rights reserved. > > Why the header? Most of our tests don't have corresponding headers. Will remove before landing. I didn't mean to include this file in the patch.
Daniel Bates
Comment 8 2018-11-28 13:55:22 PST
Committed r238635: <https://trac.webkit.org/changeset/238635>
Daniel Bates
Comment 9 2018-11-29 10:51:48 PST
Comment on attachment 355885 [details] Patch and unit tests View in context: https://bugs.webkit.org/attachment.cgi?id=355885&action=review > Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:170 > + return [m_webView isFirstResponder]; As Wenson Hsieh pointed out to me, this code is wrong as [m_webView _isRetainingActiveFocusedState] is not being considered.
Daniel Bates
Comment 10 2018-12-03 09:14:51 PST
(In reply to Daniel Bates from comment #9) > Comment on attachment 355885 [details] > Patch and unit tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=355885&action=review > > > Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:170 > > + return [m_webView isFirstResponder]; > > As Wenson Hsieh pointed out to me, this code is wrong as [m_webView > _isRetainingActiveFocusedState] is not being considered. This was fixed in the patch for bug #192165.
Note You need to log in before you can comment on or make changes to this bug.