Skip to content

Commit

Permalink
Cherry-pick 6e1c1a4. rdar://122433291
Browse files Browse the repository at this point in the history
    REGRESSION (272869@main): Actions for QR codes sometimes fail to show up when long pressing
    https://bugs.webkit.org/show_bug.cgi?id=268889
    rdar://122433291

    Reviewed by Megan Gardner.

    Fix several issues in the dynamically-inserted image analysis menu item codepath below, following
    the changes in 272869@main; importantly, this allows long-pressing ESIM and EID QR codes to show
    "Add eSIM" menu items by default in WebKit apps, such as Mail. See below for more details:

    * Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
    (-[WKContentView _insertDynamicImageAnalysisContextMenuItemsIfPossible]):

    1.  Stop looking for the "Look Up" placeholder item. Prior to 272869@main, this hidden item was
        immediately inserted into the menu if image analysis results were not available upon long press,
        and only replaced with the final image analysis menu items in the case where the client didn't
        explicitly remove the Look Up item (`_WKElementActionTypeRevealImage`). However, the "Look Up"
        item may now be added immediately (in disabled state), which replaces the placeholder item and
        causes us to return early from `-_insertDynamicImageAnalysisContextMenuItemsIfPossible` due to
        not having a placeholder to replace, even if there are MRC menu items that need to be added.

        Fix this by changing the `indexOfPlaceholderItem` to a single boolean flag, always inserting the
        dynamic items at the end of the menu, and leave out the check for the `.hidden` menu item
        attribute, so that we'll only withhold the MRC items in the case where the client explicitly
        removed the "Look Up" item (which matches existing behavior).

    2.  In a similar vein, it no longer makes sense to key the enablement of "Look Up" / "Copy Subject"
        off of whether or not the placeholder item exists, since the disabled menu item is already
        present; as such, we should remove this early return entirely and always enable these disabled
        items if necessary.

    3.  Lastly, only attempt to dynamically add the "Show Text" action in the case where "Show Text"
        wasn't already in the menu, to prevent the "Show Text" action from unnecessarily moving within
        the menu in the case where OCR finishes quickly for an image with results.

    Canonical link: https://commits.webkit.org/274222@main

Identifier: 272448.534@safari-7618.1.15.10-branch
  • Loading branch information
whsieh authored and rjepstein committed Feb 8, 2024
1 parent e4d0397 commit 22a1bc7
Showing 1 changed file with 25 additions and 24 deletions.
49 changes: 25 additions & 24 deletions Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Original file line number Diff line number Diff line change
Expand Up @@ -12397,37 +12397,28 @@ - (void)_insertDynamicImageAnalysisContextMenuItemsIfPossible
bool updated = false;
[self.contextMenuInteraction updateVisibleMenuWithBlock:makeBlockPtr([&](UIMenu *menu) -> UIMenu * {
updated = true;
__block auto indexOfPlaceholder = NSNotFound;
__block BOOL foundCopyItem = NO;
__block BOOL foundRevealImageItem = NO;
__block BOOL foundShowTextItem = NO;
auto revealImageIdentifier = elementActionTypeToUIActionIdentifier(_WKElementActionTypeRevealImage);
auto copyIdentifier = elementActionTypeToUIActionIdentifier(_WKElementActionTypeCopy);
auto showTextIdentifier = elementActionTypeToUIActionIdentifier(_WKElementActionTypeImageExtraction);
[menu.children enumerateObjectsUsingBlock:^(UIMenuElement *child, NSUInteger index, BOOL* stop) {
auto *action = dynamic_objc_cast<UIAction>(child);
if ([action.identifier isEqualToString:revealImageIdentifier] && (action.attributes & UIMenuElementAttributesHidden))
indexOfPlaceholder = index;
else if ([action.identifier isEqualToString:copyIdentifier])
foundCopyItem = YES;
if ([action.identifier isEqualToString:revealImageIdentifier])
foundRevealImageItem = YES;
else if ([action.identifier isEqualToString:showTextIdentifier])
foundShowTextItem = YES;
}];

if (indexOfPlaceholder == NSNotFound)
return menu;

auto adjustedChildren = adoptNS(menu.children.mutableCopy);
auto replacements = adoptNS([NSMutableArray<UIMenuElement *> new]);

auto *elementInfo = [_WKActivatedElementInfo activatedElementInfoWithInteractionInformationAtPosition:_positionInformation userInfo:nil];
auto addAction = [&](_WKElementActionType action) {
auto *elementAction = [_WKElementAction _elementActionWithType:action info:elementInfo assistant:_actionSheetAssistant.get()];
[replacements addObject:[elementAction uiActionForElementInfo:elementInfo]];
};
auto newItems = adoptNS([NSMutableArray<UIMenuElement *> new]);

for (UIMenuElement *child in adjustedChildren.get()) {
UIAction *action = dynamic_objc_cast<UIAction>(child);
if (!action)
continue;

if ([action.identifier isEqual:elementActionTypeToUIActionIdentifier(_WKElementActionTypeCopyCroppedImage)]) {
if (foundCopyItem && self.copySubjectResultForImageContextMenu)
if (self.copySubjectResultForImageContextMenu)
action.attributes &= ~UIMenuElementAttributesDisabled;

continue;
Expand All @@ -12441,14 +12432,24 @@ - (void)_insertDynamicImageAnalysisContextMenuItemsIfPossible
}
}

if (self.hasSelectableTextForImageContextMenu)
addAction(_WKElementActionTypeImageExtraction);
if (!foundShowTextItem && self.hasSelectableTextForImageContextMenu) {
// Dynamically insert the "Show Text" menu item, if it wasn't already inserted.
// FIXME: This should probably be inserted unconditionally, and enabled if needed
// like Look Up or Copy Subject.
auto *elementInfo = [_WKActivatedElementInfo activatedElementInfoWithInteractionInformationAtPosition:_positionInformation userInfo:nil];
auto *elementAction = [_WKElementAction _elementActionWithType:_WKElementActionTypeImageExtraction info:elementInfo assistant:_actionSheetAssistant.get()];
[newItems addObject:[elementAction uiActionForElementInfo:elementInfo]];
}

if (UIMenu *subMenu = self.machineReadableCodeSubMenuForImageContextMenu)
[replacements addObject:subMenu];
if (foundRevealImageItem) {
// Only dynamically insert machine-readable code items if the client didn't explicitly
// remove the Look Up ("reveal image") item.
if (UIMenu *subMenu = self.machineReadableCodeSubMenuForImageContextMenu)
[newItems addObject:subMenu];
}

RELEASE_LOG(ImageAnalysis, "Dynamically inserting %zu context menu action(s)", [replacements count]);
[adjustedChildren replaceObjectsInRange:NSMakeRange(indexOfPlaceholder, 1) withObjectsFromArray:replacements.get()];
RELEASE_LOG(ImageAnalysis, "Dynamically inserting %zu context menu action(s)", [newItems count]);
[adjustedChildren addObjectsFromArray:newItems.get()];
return [menu menuByReplacingChildren:adjustedChildren.get()];
}).get()];

Expand Down

0 comments on commit 22a1bc7

Please sign in to comment.