[webkit-reviews] review granted: [Bug 227265] [Live Text] [macOS] Add an internal option to disable inline text selection in images : [Attachment 431991] For EWS

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted  review:
Bug 227265: [Live Text] [macOS] Add an internal option to disable inline text
selection in images
https://bugs.webkit.org/show_bug.cgi?id=227265

Attachment 431991: For EWS

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




--- Comment #4 from Tim Horton <thorton at apple.com> ---
Comment on attachment 431991
  --> https://bugs.webkit.org/attachment.cgi?id=431991
For EWS

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

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:238
>  void WebViewImpl::computeHasVisualSearchResults(const URL& imageURL,
ShareableBitmap& imageBitmap, CompletionHandler<void(bool)>&& completion)

These seem ... slightly silly, instead of just passing the nice pretty enum.
But also fine.

> Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm:553
> +		   page->computeHasVisualSearchResults(imageURL, *imageBitmap,
[weakThis = makeWeakPtr(protectedThis.get()), quickLookItemToInsertIfNeeded =
WTFMove(*quickLookItemToInsertIfNeeded)] (bool hasVisualSearchResults) mutable
{

This function is getting a bit out of hand, maybe the visual search stuff can
be in its own? (also then you could drop a lot of the `protectedThis`)

> Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm:607
> +	   auto item = retainPtr([m_menu itemAtIndex:itemIndex]);

Not sure what the retainPtr() is for! Where's the item going to go?