[webkit-reviews] review granted: [Bug 219952] Suppress the image extraction interaction while editing text : [Attachment 416347] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 219952: Suppress the image extraction interaction while editing text
https://bugs.webkit.org/show_bug.cgi?id=219952

Attachment 416347: Patch

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




--- Comment #2 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 416347
  --> https://bugs.webkit.org/attachment.cgi?id=416347
Patch

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

r=me

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:211
> +	   if (interaction)

NIT: IMO it's a bit weird that we'd have an object that's supposed to suppress
an interaction also function if no interaction is given.  Should we
`ASSERT(interaction);`?  There's no harm in doing it as written, but it does
seem a little odd to me :P

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1297
> +    if (self._imageExtractionEnabled && (!_isBlurringFocusedElement ||
!_isChangingFocus))

Are there any other flags we should also be checking (e.g.
`_textInteractionIsHappening`)?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1298
> +	   _suppressImageExtractionToken = isEditable ?
makeUnique<WebKit::SuppressInteractionToken>(self,
_imageExtractionInteraction.get()) : nullptr;

NIT: Is the `<WebKit::SuppressInteractionToken>` necessary?  I think this can
be implicit.