Data interaction on an image should make it stand out when presenting the action...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2017 07:19:53 +0000 (07:19 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 5 Feb 2017 07:19:53 +0000 (07:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167846
<rdar://problem/30363014>

Reviewed by Tim Horton.

Adds infrastructure to improve the behavior of data interaction for images. We make two changes to accomplish
this: first, add some plumbing to WebKit so the web process can tell the UI process when it is done handling a
request to start data interaction, so that the UI process is able to clean up UI-side state in the event that
the page prevented the default behavior.

Secondly, this patch tweaks the heuristic used to present action sheets as popovers. For image elements, if
there is sufficient space around the element, we will use the element rect as the target rect; otherwise, we
fall back to presenting the popover at the touch location.

* UIProcess/PageClient.h:
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::didHandleStartDataInteractionRequest):
* UIProcess/ios/WKActionSheet.h:
* UIProcess/ios/WKActionSheet.mm:
(-[WKActionSheet presentSheet:]):

Added a presentation style parameter, used to specify whether or not WKActionSheet should present the popover
using the element rect as the target rect, or the touch location.

(-[WKActionSheet doneWithSheet]):
(-[WKActionSheet updateSheetPosition]):
(-[WKActionSheet presentSheet]): Deleted.
* UIProcess/ios/WKActionSheetAssistant.mm:
(-[WKActionSheetAssistant presentationRectForIndicatedElement]):

Returns the (inflated) bounds of the element that is currently being indicated.

(-[WKActionSheetAssistant showImageSheet]):
(-[WKActionSheetAssistant _presentationStyleForImageAtElementRect:]):
(-[WKActionSheetAssistant showLinkSheet]):
(-[WKActionSheetAssistant showDataDetectorsSheet]):
* UIProcess/ios/WKContentViewInteraction.h:
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::didHandleStartDataInteractionRequest):
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:

Remove unnecessary function declarations and implementations.

(WebKit::PageClientImpl::didPerformDataInteractionControllerOperation): Deleted.
(WebKit::PageClientImpl::startDataInteractionWithImage): Deleted.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::requestStartDataInteraction):

Notify the UI process that the web process is done handling a data interaction request, specifying whether or
not the request was granted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@211679 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/WebPageProxy.messages.in
Source/WebKit2/UIProcess/ios/PageClientImplIOS.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit2/UIProcess/ios/WKActionSheet.h
Source/WebKit2/UIProcess/ios/WKActionSheet.mm
Source/WebKit2/UIProcess/ios/WKActionSheetAssistant.mm
Source/WebKit2/UIProcess/ios/WKContentViewInteraction.h
Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm
Source/WebKit2/UIProcess/mac/PageClientImpl.h
Source/WebKit2/UIProcess/mac/PageClientImpl.mm
Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm

index 872e14d..83c1801 100644 (file)
@@ -1,3 +1,61 @@
+2017-02-04  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Data interaction on an image should make it stand out when presenting the action sheet
+        https://bugs.webkit.org/show_bug.cgi?id=167846
+        <rdar://problem/30363014>
+
+        Reviewed by Tim Horton.
+
+        Adds infrastructure to improve the behavior of data interaction for images. We make two changes to accomplish
+        this: first, add some plumbing to WebKit so the web process can tell the UI process when it is done handling a
+        request to start data interaction, so that the UI process is able to clean up UI-side state in the event that
+        the page prevented the default behavior.
+
+        Secondly, this patch tweaks the heuristic used to present action sheets as popovers. For image elements, if
+        there is sufficient space around the element, we will use the element rect as the target rect; otherwise, we
+        fall back to presenting the popover at the touch location.
+
+        * UIProcess/PageClient.h:
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::didHandleStartDataInteractionRequest):
+        * UIProcess/ios/WKActionSheet.h:
+        * UIProcess/ios/WKActionSheet.mm:
+        (-[WKActionSheet presentSheet:]):
+
+        Added a presentation style parameter, used to specify whether or not WKActionSheet should present the popover
+        using the element rect as the target rect, or the touch location.
+
+        (-[WKActionSheet doneWithSheet]):
+        (-[WKActionSheet updateSheetPosition]):
+        (-[WKActionSheet presentSheet]): Deleted.
+        * UIProcess/ios/WKActionSheetAssistant.mm:
+        (-[WKActionSheetAssistant presentationRectForIndicatedElement]):
+
+        Returns the (inflated) bounds of the element that is currently being indicated.
+
+        (-[WKActionSheetAssistant showImageSheet]):
+        (-[WKActionSheetAssistant _presentationStyleForImageAtElementRect:]):
+        (-[WKActionSheetAssistant showLinkSheet]):
+        (-[WKActionSheetAssistant showDataDetectorsSheet]):
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::didHandleStartDataInteractionRequest):
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+
+        Remove unnecessary function declarations and implementations.
+
+        (WebKit::PageClientImpl::didPerformDataInteractionControllerOperation): Deleted.
+        (WebKit::PageClientImpl::startDataInteractionWithImage): Deleted.
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::requestStartDataInteraction):
+
+        Notify the UI process that the web process is done handling a data interaction request, specifying whether or
+        not the request was granted.
+
 2017-02-04  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. Fix the key shortcut to enable resource usage overlay in GTK+.
index 10a1c86..06d33c7 100644 (file)
@@ -383,6 +383,7 @@ public:
 
 #if ENABLE(DATA_INTERACTION)
     virtual void didPerformDataInteractionControllerOperation() = 0;
+    virtual void didHandleStartDataInteractionRequest(bool started) = 0;
     virtual void startDataInteractionWithImage(const WebCore::IntPoint& clientPosition, const ShareableBitmap::Handle& image, const WebCore::FloatPoint& anchorPoint, bool isLink) = 0;
 #endif
 };
index b8abf50..1ad134d 100644 (file)
@@ -553,6 +553,7 @@ public:
     void requestRectsAtSelectionOffsetWithText(int32_t offset, const String&, std::function<void(const Vector<WebCore::SelectionRect>&, CallbackBase::Error)>);
 #if ENABLE(DATA_INTERACTION)
     void didPerformDataInteractionControllerOperation();
+    void didHandleStartDataInteractionRequest(bool started);
     void requestStartDataInteraction(const WebCore::IntPoint& clientPosition, const WebCore::IntPoint& globalPosition);
 #endif
 #endif
index 9df3025..78c3975 100644 (file)
@@ -319,6 +319,7 @@ messages -> WebPageProxy {
 
 #if ENABLE(DATA_INTERACTION)
     DidPerformDataInteractionControllerOperation()
+    DidHandleStartDataInteractionRequest(bool started)
 #endif
 
 #if PLATFORM(COCOA)
index c407e8c..f72b752 100644 (file)
@@ -203,6 +203,7 @@ private:
 
 #if ENABLE(DATA_INTERACTION)
     void didPerformDataInteractionControllerOperation() override;
+    void didHandleStartDataInteractionRequest(bool started) override;
     void startDataInteractionWithImage(const WebCore::IntPoint& clientPosition, const ShareableBitmap::Handle& image, const WebCore::FloatPoint& anchorPoint, bool isLink) override;
 #endif
 
index 1831250..b910d0d 100644 (file)
@@ -768,6 +768,11 @@ void PageClientImpl::didPerformDataInteractionControllerOperation()
     [m_contentView _didPerformDataInteractionControllerOperation];
 }
 
+void PageClientImpl::didHandleStartDataInteractionRequest(bool started)
+{
+    [m_contentView _didHandleStartDataInteractionRequest:started];
+}
+
 void PageClientImpl::startDataInteractionWithImage(const IntPoint& clientPosition, const ShareableBitmap::Handle& image, const FloatPoint& anchorPoint, bool isLink)
 {
     [m_contentView _startDataInteractionWithImage:ShareableBitmap::create(image)->makeCGImageCopy() atClientPosition:CGPointMake(clientPosition.x(), clientPosition.y()) anchorPoint:anchorPoint isLink:isLink];
index 1a2c07b..05efae0 100644 (file)
 #import <UIKit/UIAlertController.h>
 #import <UIKit/UIPopoverController.h>
 
+typedef NS_ENUM(NSInteger, WKActionSheetPresentationStyle) {
+    WKActionSheetPresentAtTouchLocation,
+    WKActionSheetPresentAtElementRect
+};
+
 @protocol WKActionSheetDelegate;
 @class WKContentView;
 
@@ -36,7 +41,7 @@
 @property (nonatomic, assign) id <WKActionSheetDelegate> sheetDelegate;
 @property (nonatomic) UIPopoverArrowDirection arrowDirections;
 - (void)doneWithSheet;
-- (BOOL)presentSheet;
+- (BOOL)presentSheet:(WKActionSheetPresentationStyle)style;
 - (BOOL)presentSheetFromRect:(CGRect)presentationRect;
 - (void)updateSheetPosition;
 @end
@@ -45,6 +50,7 @@
 @required
 - (UIView *)hostViewForSheet;
 - (CGRect)initialPresentationRectInHostViewForSheet;
+- (CGRect)presentationRectForIndicatedElement;
 - (CGRect)presentationRectInHostViewForSheet;
 - (void)updatePositionInformation;
 @end
index 3c8252d..aa899d6 100644 (file)
@@ -37,6 +37,7 @@
     BOOL _isRotating;
     BOOL _readyToPresentAfterRotation;
 
+    WKActionSheetPresentationStyle _currentPresentationStyle;
     RetainPtr<UIViewController> _currentPresentingViewController;
     RetainPtr<UIViewController> _presentedViewControllerWhileRotating;
     RetainPtr<id <UIPopoverPresentationControllerDelegate>> _popoverPresentationControllerDelegateWhileRotating;
 
 #pragma mark - Sheet presentation code
 
-- (BOOL)presentSheet
+- (BOOL)presentSheet:(WKActionSheetPresentationStyle)style
 {
     // Calculate the presentation rect just before showing.
     CGRect presentationRect = CGRectZero;
     if (UI_USER_INTERFACE_IDIOM() != UIUserInterfaceIdiomPhone) {
-        presentationRect = [_sheetDelegate initialPresentationRectInHostViewForSheet];
+        presentationRect = style == WKActionSheetPresentAtElementRect ? [_sheetDelegate presentationRectForIndicatedElement] : [_sheetDelegate initialPresentationRectInHostViewForSheet];
         if (CGRectIsEmpty(presentationRect))
             return NO;
     }
 
+    _currentPresentationStyle = style;
     return [self presentSheetFromRect:presentationRect];
 }
 
     _currentPresentingViewController = nil;
     _presentedViewControllerWhileRotating = nil;
     _popoverPresentationControllerDelegateWhileRotating = nil;
+    _currentPresentationStyle = WKActionSheetPresentAtTouchLocation;
 
     [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(_didRotateAndLayout) object:nil];
 }
     if (_isRotating || !_readyToPresentAfterRotation || isBeingPresented)
         return;
 
-    CGRect presentationRect = [_sheetDelegate initialPresentationRectInHostViewForSheet];
+    CGRect presentationRect = _currentPresentationStyle == WKActionSheetPresentAtElementRect ? [_sheetDelegate presentationRectForIndicatedElement] : [_sheetDelegate initialPresentationRectInHostViewForSheet];
     BOOL wasPresentedViewControllerModal = [_presentedViewControllerWhileRotating isModalInPopover];
 
     if (!CGRectIsEmpty(presentationRect) || wasPresentedViewControllerModal) {
         if (!CGRectIsEmpty(intersection))
             [self presentSheetFromRect:intersection];
         else if (wasPresentedViewControllerModal)
-            [self presentSheet];
+            [self presentSheet:_currentPresentationStyle];
 
         _presentedViewControllerWhileRotating = nil;
         _popoverPresentationControllerDelegateWhileRotating = nil;
index b5a1a20..315d7b4 100644 (file)
@@ -152,6 +152,18 @@ static LSAppLink *appLinkForURL(NSURL *url)
     return [self superviewForSheet];
 }
 
+- (CGRect)presentationRectForIndicatedElement
+{
+    UIView *view = [self superviewForSheet];
+    auto delegate = _delegate.get();
+    if (!view || !delegate)
+        return CGRectZero;
+
+    static const CGFloat presentationElementRectPadding = 15;
+    auto elementBounds = [delegate positionInformationForActionSheetAssistant:self].bounds;
+    return CGRectInset([view convertRect:elementBounds fromView:_view], -presentationElementRectPadding, -presentationElementRectPadding);
+}
+
 - (CGRect)initialPresentationRectInHostViewForSheet
 {
     UIView *view = [self superviewForSheet];
@@ -293,7 +305,8 @@ static LSAppLink *appLinkForURL(NSURL *url)
     const auto& positionInformation = [delegate positionInformationForActionSheetAssistant:self];
 
     NSURL *targetURL = [NSURL _web_URLWithWTFString:positionInformation.url];
-    auto elementInfo = adoptNS([[_WKActivatedElementInfo alloc] _initWithType:_WKActivatedElementTypeImage URL:targetURL location:positionInformation.request.point title:positionInformation.title ID:positionInformation.idAttribute rect:positionInformation.bounds image:positionInformation.image.get()]);
+    auto elementBounds = positionInformation.bounds;
+    auto elementInfo = adoptNS([[_WKActivatedElementInfo alloc] _initWithType:_WKActivatedElementTypeImage URL:targetURL location:positionInformation.request.point title:positionInformation.title ID:positionInformation.idAttribute rect:elementBounds image:positionInformation.image.get()]);
     if ([delegate respondsToSelector:@selector(actionSheetAssistant:showCustomSheetForElement:)] && [delegate actionSheetAssistant:self showCustomSheetForElement:elementInfo.get()])
         return;
     auto defaultActions = [self defaultActionsForImageSheet:elementInfo.get()];
@@ -309,10 +322,30 @@ static LSAppLink *appLinkForURL(NSURL *url)
 
     _elementInfo = WTFMove(elementInfo);
 
-    if (![_interactionSheet presentSheet])
+    if (![_interactionSheet presentSheet:[self _presentationStyleForImageAtElementRect:elementBounds]])
         [self cleanupSheet];
 }
 
+- (WKActionSheetPresentationStyle)_presentationStyleForImageAtElementRect:(CGRect)elementRect
+{
+    auto apparentElementRect = [_view convertRect:elementRect toView:_view.window];
+    auto windowRect = _view.window.bounds;
+    apparentElementRect = CGRectIntersection(apparentElementRect, windowRect);
+
+    auto leftInset = CGRectGetMinX(apparentElementRect) - CGRectGetMinX(windowRect);
+    auto topInset = CGRectGetMinY(apparentElementRect) - CGRectGetMinY(windowRect);
+    auto rightInset = CGRectGetMaxX(windowRect) - CGRectGetMaxX(apparentElementRect);
+    auto bottomInset = CGRectGetMaxY(windowRect) - CGRectGetMaxY(apparentElementRect);
+
+    // If at least this much of the window is available for the popover to draw in, then target the element rect when presenting the action menu popover.
+    // 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;
+    if (std::max(leftInset, rightInset) > minimumAvailableWidthOrHeightRatio * CGRectGetWidth(windowRect) || std::max(topInset, bottomInset) > minimumAvailableWidthOrHeightRatio * CGRectGetHeight(windowRect))
+        return WKActionSheetPresentAtElementRect;
+
+    return WKActionSheetPresentAtTouchLocation;
+}
+
 - (void)_appendOpenActionsForURL:(NSURL *)url actions:(NSMutableArray *)defaultActions elementInfo:(_WKActivatedElementInfo *)elementInfo
 {
 #if HAVE(APP_LINKS)
@@ -428,7 +461,7 @@ static LSAppLink *appLinkForURL(NSURL *url)
 
     _elementInfo = WTFMove(elementInfo);
 
-    if (![_interactionSheet presentSheet])
+    if (![_interactionSheet presentSheet:WKActionSheetPresentAtTouchLocation])
         [self cleanupSheet];
 }
 
@@ -494,7 +527,7 @@ static LSAppLink *appLinkForURL(NSURL *url)
     if (elementActions.count <= 1)
         _interactionSheet.get().arrowDirections = UIPopoverArrowDirectionUp | UIPopoverArrowDirectionDown;
 
-    if (![_interactionSheet presentSheet])
+    if (![_interactionSheet presentSheet:WKActionSheetPresentAtTouchLocation])
         [self cleanupSheet];
 }
 
index 76e6551..e0564af 100644 (file)
@@ -252,6 +252,7 @@ struct WKAutoCorrectionData {
 
 #if ENABLE(DATA_INTERACTION)
 - (void)_didPerformDataInteractionControllerOperation;
+- (void)_didHandleStartDataInteractionRequest:(BOOL)started;
 - (void)_startDataInteractionWithImage:(RetainPtr<CGImageRef>)image atClientPosition:(CGPoint)clientPosition anchorPoint:(CGPoint)anchorPoint isLink:(BOOL)isLink;
 #endif
 
index e97985f..fb4ca0d 100644 (file)
@@ -1121,6 +1121,11 @@ void WebPageProxy::didPerformDataInteractionControllerOperation()
     m_pageClient.didPerformDataInteractionControllerOperation();
 }
 
+void WebPageProxy::didHandleStartDataInteractionRequest(bool started)
+{
+    m_pageClient.didHandleStartDataInteractionRequest(started);
+}
+
 void WebPageProxy::requestStartDataInteraction(const WebCore::IntPoint& clientPosition, const WebCore::IntPoint& globalPosition)
 {
     if (isValid())
index fee9c54..0d2ddaa 100644 (file)
@@ -232,11 +232,6 @@ private:
     _WKRemoteObjectRegistry *remoteObjectRegistry() override;
 #endif
 
-#if ENABLE(DATA_INTERACTION)
-    void didPerformDataInteractionControllerOperation() override;
-    void startDataInteractionWithImage(const WebCore::IntPoint& clientPosition, const ShareableBitmap::Handle& image, const WebCore::FloatPoint& anchorPoint, bool isLink) override;
-#endif
-
     NSView *m_view;
     WKWebView *m_webView;
     WebViewImpl* m_impl { nullptr };
index 18faf9b..b0e4676 100644 (file)
@@ -875,18 +875,6 @@ bool PageClientImpl::windowIsFrontWindowUnderMouse(const NativeWebMouseEvent& ev
     return m_impl->windowIsFrontWindowUnderMouse(event.nativeEvent());
 }
 
-#if ENABLE(DATA_INTERACTION)
-void PageClientImpl::didPerformDataInteractionControllerOperation()
-{
-    // FIXME: Implement me.
-}
-
-void PageClientImpl::startDataInteractionWithImage(const IntPoint&, const ShareableBitmap::Handle&, const FloatPoint&, bool)
-{
-    // FIXME: Implement me.
-}
-#endif
-
 WebCore::UserInterfaceLayoutDirection PageClientImpl::userInterfaceLayoutDirection()
 {
     if (!m_view)
index 2d95043..99f7b90 100644 (file)
@@ -625,7 +625,8 @@ void WebPage::handleTap(const IntPoint& point, uint64_t lastLayerTreeTransaction
 #if ENABLE(DATA_INTERACTION)
 void WebPage::requestStartDataInteraction(const IntPoint& clientPosition, const IntPoint& globalPosition)
 {
-    m_page->mainFrame().eventHandler().tryToBeginDataInteractionAtPoint(clientPosition, globalPosition);
+    bool didStart = m_page->mainFrame().eventHandler().tryToBeginDataInteractionAtPoint(clientPosition, globalPosition);
+    send(Messages::WebPageProxy::DidHandleStartDataInteractionRequest(didStart));
 }
 #endif