Changeset 284889 in webkit

Timestamp:

Oct 26, 2021, 12:44:14 PM (5 years ago)

Author:

Wenson Hsieh

Message:

REGRESSION (r281054): [iOS] Context menu presents from wrong location when long pressing a link in Mail
​https://bugs.webkit.org/show_bug.cgi?id=232287
rdar://82671325

Reviewed by Tim Horton.

In the case where the WebKit client isn't overriding the context menu configuration via WebKit context menu UI
delegate methods, _contextMenuElementInfo on WKContentView will end up being nil while presenting the context
menu via long press.

After the changes in r281054, this means that when the last view is removed from our WKTargetedPreviewContainer,
we'll unparent WKTargetedPreviewContainer too early, since _contextMenuElementInfo won't prevent us from
bailing in -_removeContextMenuHintContainerIfPossible. To fix this, we add a boolean flag to track when the
context menu presentation animation is running, and avoid unparenting the preview container if the flag is set.

• UIProcess/ios/WKContentViewInteraction.h:
• UIProcess/ios/WKContentViewInteraction.mm:

(-[WKContentView setUpInteraction]):
(-[WKContentView _removeContextMenuHintContainerIfPossible]):
(-[WKContentView _contentsOfUserInterfaceItem:]):
(-[WKContentView contextMenuInteraction:willDisplayMenuForConfiguration:animator:]):

To test this change, add an assertion that fires if the context menu preview hint container has already been
unparented by the time we've presented the context menu. This assertion already fires during the extant layout
test fast/events/touch/ios/long-press-on-link.html, which technically exhibits the bug (albeit in a more subtle
way).

(-[WKContentView contextMenuInteraction:willEndForConfiguration:animator:]):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ios/WKContentViewInteraction.h (modified) (1 diff)
• UIProcess/ios/WKContentViewInteraction.mm (modified) (6 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-10-26  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION (r281054): [iOS] Context menu presents from wrong location when long pressing a link in Mail
+        https://bugs.webkit.org/show_bug.cgi?id=232287
+        rdar://82671325
+
+        Reviewed by Tim Horton.
+
+        In the case where the WebKit client isn't overriding the context menu configuration via WebKit context menu UI
+        delegate methods, `_contextMenuElementInfo` on WKContentView will end up being nil while presenting the context
+        menu via long press.
+
+        After the changes in r281054, this means that when the last view is removed from our WKTargetedPreviewContainer,
+        we'll unparent WKTargetedPreviewContainer too early, since `_contextMenuElementInfo` won't prevent us from
+        bailing in `-_removeContextMenuHintContainerIfPossible`. To fix this, we add a boolean flag to track when the
+        context menu presentation animation is running, and avoid unparenting the preview container if the flag is set.
+
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView setUpInteraction]):
+        (-[WKContentView _removeContextMenuHintContainerIfPossible]):
+        (-[WKContentView _contentsOfUserInterfaceItem:]):
+        (-[WKContentView contextMenuInteraction:willDisplayMenuForConfiguration:animator:]):
+
+        To test this change, add an assertion that fires if the context menu preview hint container has already been
+        unparented by the time we've presented the context menu. This assertion already fires during the extant layout
+        test fast/events/touch/ios/long-press-on-link.html, which technically exhibits the bug (albeit in a more subtle
+        way).
+
+        (-[WKContentView contextMenuInteraction:willEndForConfiguration:animator:]):
+
 2021-10-26  Brady Eidson  <beidson@apple.com>
 

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h

@@ -351 +351 @@
     BOOL _contextMenuHasRequestedLegacyData;
     BOOL _contextMenuActionProviderDelegateNeedsOverride;
+    BOOL _isDisplayingContextMenuWithAnimation;
 #endif
     RetainPtr<UIPreviewItemController> _previewItemController;

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

@@ -1029 +1029 @@
     _isChangingFocus = NO;
     _isBlurringFocusedElement = NO;
+#if USE(UICONTEXTMENU)
+    _isDisplayingContextMenuWithAnimation = NO;
+#endif
 
 #if USE(UICONTEXTMENU) && ENABLE(IMAGE_ANALYSIS)
@@ -8626 +8629 @@
         return;
 #endif
+    if (_isDisplayingContextMenuWithAnimation)
+        return;
 #if ENABLE(DATA_DETECTION)
     // We are also using this container for the action sheet assistant...
@@ -10525 +10530 @@
 #if HAVE(LINK_PREVIEW)
     if ([userInterfaceItem isEqualToString:@"contextMenu"]) {
-        if (self._shouldUseContextMenus)
+        if (self._shouldUseContextMenus) {
             return @{ userInterfaceItem: @{
                 @"url": _positionInformation.url.isValid() ? WTF::userVisibleString(_positionInformation.url) : @"",
@@ -10532 +10537 @@
                 @"imageURL": _positionInformation.imageURL.isValid() ? WTF::userVisibleString(_positionInformation.imageURL) : @""
             } };
+        }
         NSString *url = [_previewItemController previewData][UIPreviewDataLink];
         return @{ userInterfaceItem: @{
@@ -11133 +11139 @@
     if (!_webView)
         return;
+
+    _isDisplayingContextMenuWithAnimation = YES;
+    [animator addCompletion:[weakSelf = WeakObjCPtr<WKContentView>(self)] {
+        if (auto strongSelf = weakSelf.get()) {
+            ASSERT_IMPLIES(strongSelf->_isDisplayingContextMenuWithAnimation, [strongSelf->_contextMenuHintContainerView window]);
+            strongSelf->_isDisplayingContextMenuWithAnimation = NO;
+        }
+    }];
+
     auto uiDelegate = static_cast<id<WKUIDelegatePrivate>>(self.webView.UIDelegate);
     if (!uiDelegate)
         return;
+
     if ([uiDelegate respondsToSelector:@selector(webView:contextMenuWillPresentForElement:)])
         [uiDelegate webView:self.webView contextMenuWillPresentForElement:_contextMenuElementInfo.get()];
@@ -11294 +11310 @@
         if (!strongSelf)
             return;
+
+        strongSelf->_isDisplayingContextMenuWithAnimation = NO;
         [strongSelf _removeContextMenuHintContainerIfPossible];
         [strongSelf->_webView _didDismissContextMenu];