[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 bugzilla-daemon at webkit.org
Tue Jun 22 14:18:20 PDT 2021

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

Attachment 431991: For EWS


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

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?

More information about the webkit-reviews mailing list