Bug 192084 – [iOS] Page not defocused when Find-in-page becomes first responder

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Daniel Bates]

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.

[Daniel Bates]

<rdar://problem/46146777>

[Daniel Bates]

Created attachment 355884 [details]
Patch and unit tests

[Daniel Bates]

Created attachment 355885 [details]
Patch and unit tests

[Tim Horton]

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 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]

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]

(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]

Committed r238635: <https://trac.webkit.org/changeset/238635>

[Daniel Bates]

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]

(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.