Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] The "Copy Cropped Image" context menu action should be gated on cropped image results #469

Merged
merged 0 commits into from May 3, 2022

Conversation

whsieh
Copy link
Member

@whsieh whsieh commented May 3, 2022

df8306e

[iOS] The "Copy Cropped Image" context menu action should be gated on cropped image results
https://bugs.webkit.org/show_bug.cgi?id=240013
rdar://88941787

Reviewed by Tim Horton.

Only show this item in the context menu when long pressing in the case where `requestImageAnalysisMarkup`
computes a non-null result for the given image bitmap. This gating logic runs alongside existing gating logic
for both the visual search item ("Look Up") and "Show Text" actions. See below for more details.

* UIProcess/ios/WKContentViewInteraction.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _setUpImageAnalysis]):
(-[WKContentView _tearDownImageAnalysis]):

Introduce a `_croppedImageResult` ivar to cache the CGImage result after running image analysis over the image
corresponding to the element for which we're showing the context menu. This is reset in the same lifecycle as
the extant `_hasVisualSearchResults` and `_hasSelectableTextInImage` flags which are used for the same purpose.

(-[WKContentView imageAnalysisGestureDidBegin:]):
(-[WKContentView _completeImageAnalysisRequestForContextMenu:requestIdentifier:hasTextResults:]):

This is the codepath that currently prevents us from showing the context menu until we know whether or not there
are relevant visual search results, such that we can conditionally show the "Look Up" context menu item. Adjust
this so that it calls `-_invokeAllActionsToPerformAfterPendingImageAnalysis:` only after we've also determined
whether or not there is a non-null cropped image result, so that we can also conditionally show the "Copy
Cropped Image" item.

To achieve this, we move the call to `-_invokeAllActionsToPerformAfterPendingImageAnalysis:` into a
`WTF::CallbackAggregator`, and ref/deref the aggregator when invoking both of the async image analysis
operations. When the callback aggregator is destroyed (i.e., after both async image analysis operations are
complete), we proceed with showing the context menu.

(-[WKContentView imageAnalysisGestureDidTimeOut:]):

Implement similar logic as above, but for the scenario where we show the context menu after the user continues
to long press after selecting text inside of an image.

(-[WKContentView actionSheetAssistantShouldIncludeCopyCroppedImageAction:]):

Only show the item if `_croppedImageResult` is non-null.

(-[WKContentView actionSheetAssistant:copyCroppedImage:sourceMIMEType:]):

Instead of running image analysis and writing the resulting image to the clipboard, simply transcode the cached
image in `_croppedImageResult`.

Canonical link: https://commits.webkit.org/250218@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293730 268f45cc-cd09-0410-ab3c-d52691b4dbfc

@whsieh whsieh self-assigned this May 3, 2022
@whsieh whsieh added Platform Portability improvements and other general platform improvements not driven directly by site bugs. WebKit Nightly Build labels May 3, 2022
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label May 3, 2022
@whsieh whsieh removed merging-blocked Applied to prevent a change from being merged Platform Portability improvements and other general platform improvements not driven directly by site bugs. WebKit Nightly Build labels May 3, 2022
@whsieh whsieh added Platform Portability improvements and other general platform improvements not driven directly by site bugs. WebKit Nightly Build labels May 3, 2022
@whsieh whsieh added the merge-queue Applied to send a pull request to merge-queue label May 3, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit df8306e into WebKit:main May 3, 2022
@webkit-early-warning-system
Copy link
Collaborator

Committed r293730 (250218@main): https://commits.webkit.org/250218@main

Reviewed commits have been landed. Closing PR #469 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system removed the merge-queue Applied to send a pull request to merge-queue label May 3, 2022
@whsieh whsieh deleted the eng/240013 branch May 4, 2022 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform Portability improvements and other general platform improvements not driven directly by site bugs.
Projects
None yet
3 participants