Changeset 219287 in webkit

Timestamp:

Jul 10, 2017 3:04:53 AM (7 years ago)

Author:

Wenson Hsieh

Message:

[WK2] Action sheets for links fail to present in WebKit2 PDF view
​https://bugs.webkit.org/show_bug.cgi?id=174307
<rdar://problem/31412128>

Reviewed by Tim Horton.

Currently, presenting an action sheet for a link always uses the WKActionSheetPresentAtClosestIndicatorRect
codepath, which requires text indicator data for the link. However, when showing an action sheet for a link via
WKPDFView, a text indicator for the link is not included, so the popover rect ends up being an empty rect at the
origin, which causes us to bail from presenting the popover.

To address this, we tweak our heuristic for determining which action sheet presentation style to use, so that we
only use the closest indicator rect for a link if the text indicator data is also present (otherwise, we fall
back to using the element rect). All other behavior is the same.

• UIProcess/ios/WKActionSheetAssistant.mm:

(-[WKActionSheetAssistant showImageSheet]):
(presentationStyleForView):

Refactor _shouldPresentAtTouchLocationForElementRect into presentationStyleForView, a static function that
returns a WKActionSheetPresentationStyle.

(-[WKActionSheetAssistant showLinkSheet]):
(-[WKActionSheetAssistant _shouldPresentAtTouchLocationForElementRect:]): Deleted.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ios/WKActionSheetAssistant.mm (modified) (4 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2017-07-10  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [WK2] Action sheets for links fail to present in WebKit2 PDF view
+        https://bugs.webkit.org/show_bug.cgi?id=174307
+        <rdar://problem/31412128>
+
+        Reviewed by Tim Horton.
+
+        Currently, presenting an action sheet for a link always uses the WKActionSheetPresentAtClosestIndicatorRect
+        codepath, which requires text indicator data for the link. However, when showing an action sheet for a link via
+        WKPDFView, a text indicator for the link is not included, so the popover rect ends up being an empty rect at the
+        origin, which causes us to bail from presenting the popover.
+
+        To address this, we tweak our heuristic for determining which action sheet presentation style to use, so that we
+        only use the closest indicator rect for a link if the text indicator data is also present (otherwise, we fall
+        back to using the element rect). All other behavior is the same.
+
+        * UIProcess/ios/WKActionSheetAssistant.mm:
+        (-[WKActionSheetAssistant showImageSheet]):
+        (presentationStyleForView):
+
+        Refactor _shouldPresentAtTouchLocationForElementRect into presentationStyleForView, a static function that
+        returns a WKActionSheetPresentationStyle.
+
+        (-[WKActionSheetAssistant showLinkSheet]):
+        (-[WKActionSheetAssistant _shouldPresentAtTouchLocationForElementRect:]): Deleted.
+
 2017-07-09  Brady Eidson  <beidson@apple.com>
 

trunk/Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm

@@ -372 +372 @@
         _elementInfo = WTFMove(elementInfo);
 
-        if (![_interactionSheet presentSheet:[self _shouldPresentAtTouchLocationForElementRect:elementBounds] ? WKActionSheetPresentAtTouchLocation : WKActionSheetPresentAtElementRect])
+        if (![_interactionSheet presentSheet:presentationStyleForView(_view.getAutoreleased(), _positionInformation.value(), _elementInfo.get())])
             [self cleanupSheet];
     };
@@ -394 +394 @@
 }
 
-- (BOOL)_shouldPresentAtTouchLocationForElementRect:(CGRect)elementRect
-{
-    UIView *view = _view.getAutoreleased();
-    auto apparentElementRect = [view convertRect:elementRect toView:view.window];
+static WKActionSheetPresentationStyle presentationStyleForView(UIView *view, const InteractionInformationAtPosition& positionInfo, _WKActivatedElementInfo *elementInfo)
+{
+    auto apparentElementRect = [view convertRect:positionInfo.bounds toView:view.window];
     auto windowRect = view.window.bounds;
     apparentElementRect = CGRectIntersection(apparentElementRect, windowRect);
@@ -409 +408 @@
     // Otherwise, there is not enough space to position the popover around the element, so revert to using the touch location instead.
     static const CGFloat minimumAvailableWidthOrHeightRatio = 0.4;
-    return std::max(leftInset, rightInset) <= minimumAvailableWidthOrHeightRatio * CGRectGetWidth(windowRect) && std::max(topInset, bottomInset) <= minimumAvailableWidthOrHeightRatio * CGRectGetHeight(windowRect);
+    if (std::max(leftInset, rightInset) <= minimumAvailableWidthOrHeightRatio * CGRectGetWidth(windowRect) && std::max(topInset, bottomInset) <= minimumAvailableWidthOrHeightRatio * CGRectGetHeight(windowRect))
+        return WKActionSheetPresentAtTouchLocation;
+
+    if (elementInfo.type == _WKActivatedElementTypeLink && positionInfo.linkIndicator.textRectsInBoundingRectCoordinates.size())
+        return WKActionSheetPresentAtClosestIndicatorRect;
+
+    return WKActionSheetPresentAtElementRect;
 }
 
@@ -531 +536 @@
     _elementInfo = WTFMove(elementInfo);
 
-    if (![_interactionSheet presentSheet:[self _shouldPresentAtTouchLocationForElementRect:_positionInformation->bounds] ? WKActionSheetPresentAtTouchLocation : WKActionSheetPresentAtClosestIndicatorRect])
+    if (![_interactionSheet presentSheet:presentationStyleForView(_view.getAutoreleased(), _positionInformation.value(), _elementInfo.get())])
         [self cleanupSheet];
 }