[webkit-reviews] review granted: [Bug 192165] REGRESSION (r238635): Dragging a text selection within WKWebView causes the selection highlight to get into a bad state : [Attachment 356107] Fix open source build

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 192165: REGRESSION (r238635): Dragging a text selection within WKWebView
causes the selection highlight to get into a bad state
https://bugs.webkit.org/show_bug.cgi?id=192165

Attachment 356107: Fix open source build

https://bugs.webkit.org/attachment.cgi?id=356107&action=review




--- Comment #7 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 356107
  --> https://bugs.webkit.org/attachment.cgi?id=356107
Fix open source build

View in context: https://bugs.webkit.org/attachment.cgi?id=356107&action=review

How do we handle the case where the drag is cancelled? Does _conclude* get
called with the source WebView such that we make it first responder agin. Now
that we have balanced resigning and making first responder for drag and drop do
we still need the concept of “retaining active focus state” concept?

> Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:169
> +    if ([m_webView _isRetainingActiveFocusedState])

The checking of this condition first makes it asymmetric to where we check it
in isViewWindowActive(). It would be good to either fix this code or fix
isViewWindowActice() such that they have symmetry. If we choose to keep the
order the way it is here then I would suggest flattening this if statement into
a disjunction that is part of the return statement to avoid a branch (though
the compiler is likely smart enough to do this for us :/). On another note and
one that can be addressed in another patch, I find this “retaining active
focused state” concept mysterious. If this is only used for drag and drop then
it would be good to rename this to reflect that. Otherwise, I suggest we add a
comment to provide examples of when this state is set.

> Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:467
> +	   [_webView becomeFirstResponder];

I am assuming _webView is the drop destination.

> Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:555
> +	   [_webView resignFirstResponder];

I am assuming _webView is the source WebView.