Bug 192084 - [iOS] Page not defocused when Find-in-page becomes first responder
Summary: [iOS] Page not defocused when Find-in-page becomes first responder
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 12
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2018-11-28 09:52 PST by Daniel Bates
Modified: 2018-12-03 09:14 PST (History)
6 users (show)

See Also:


Attachments
Patch and unit tests (16.89 KB, patch)
2018-11-28 10:06 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch and unit tests (16.86 KB, patch)
2018-11-28 10:11 PST, Daniel Bates
thorton: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
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

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 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.
Comment 1 Daniel Bates 2018-11-28 09:52:35 PST
<rdar://problem/46146777>
Comment 2 Daniel Bates 2018-11-28 10:06:42 PST
Created attachment 355884 [details]
Patch and unit tests
Comment 3 Daniel Bates 2018-11-28 10:11:48 PST
Created attachment 355885 [details]
Patch and unit tests
Comment 4 Tim Horton 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.
Comment 5 EWS Watchlist 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.
Comment 6 EWS Watchlist 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
Comment 7 Daniel Bates 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.
Comment 8 Daniel Bates 2018-11-28 13:55:22 PST
Committed r238635: <https://trac.webkit.org/changeset/238635>
Comment 9 Daniel Bates 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.
Comment 10 Daniel Bates 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.