[webkit-reviews] review granted: [Bug 210230] Popovers are dismissed immediately when they try and bring up the keyboard. : [Attachment 395890] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 210230: Popovers are dismissed immediately when they try and bring up the
keyboard.
https://bugs.webkit.org/show_bug.cgi?id=210230

Attachment 395890: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 395890
  --> https://bugs.webkit.org/attachment.cgi?id=395890
Patch

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

Any way to make a test for this?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6669
> +- (void)preserveFocus
> +{
> +    [_webView _incrementFocusPreservationCount];
> +}
> +
> +- (void)releaseFocus
> +{
> +    [_webView _decrementFocusPreservationCount];
> +}

Is there any risk that a WKContentView could be deallocated before releaseFocus
is called, or have _webView set to null or any new value before this is called?

Often, designs like this one can be dangerous if a stale count can be left
behind. There are techniques to make them more resilient, like using objects
and weak pointers, that are most costly but more foolproof.

> Source/WebKit/UIProcess/ios/forms/WKFormInputControl.mm:341
> +	   if (_view.focusedElementInformation.elementType == InputType::Time
|| _view.focusedElementInformation.elementType == InputType::DateTimeLocal)
> +	       [_view releaseFocus];

Maybe we could use a boolean _preservingFocus rather than repeating this check
here. That way there’s no risk of the check in controlBeginEditing being out of
sync with this one.