Bug 155205 - Add two finger tap on links
Summary: Add two finger tap on links
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-08 16:22 PST by Enrica Casucci
Modified: 2016-03-09 11:29 PST (History)
2 users (show)

See Also:


Attachments
Patch (11.38 KB, patch)
2016-03-08 16:29 PST, Enrica Casucci
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2016-03-08 16:22:57 PST
Adding new gesture to let the delegate implement alternate actions.
Comment 1 Enrica Casucci 2016-03-08 16:26:39 PST
rdar://problem/22937516
Comment 2 Enrica Casucci 2016-03-08 16:29:43 PST
Created attachment 273364 [details]
Patch
Comment 3 Sam Weinig 2016-03-08 18:13:40 PST
Comment on attachment 273364 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=273364&action=review

r=me, but you need to add the things to UIKitSPI.h to fix the build and I think you should consider my comments.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:490
> +    [_twoFingerSingleTapGestureRecognizer setAllowableMovement:60];

You probably need to add this to UIKitSPI.h. Also, what is this in units of?

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:493
> +    [_twoFingerSingleTapGestureRecognizer setDelaysTouchesEnded :NO];

Weird space after ended.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1315
> +    _page->handleTwoFingerTapAtPoint(roundedIntPoint(gestureRecognizer.centroid), [view, webView](const String& string, CallbackBase::Error error) {

You probably need to add [UITapGestureRecognizer centroid] to UIKitSPI.h

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1321
> +            if ([uiDelegate respondsToSelector:@selector(_webView:alternateActionForURL:)])
> +                [uiDelegate _webView:webView alternateActionForURL:[NSURL _web_URLWithWTFString:string]];

Instead of passing a URL here, could we pass an _WKActivatedElementInfo (or subclass of it), and let the client make the decision?  That way, if someone wants to do something with images, or something else, they could.  It would also allow the client to show that something was happening, because the boundingRect would be available.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:633
> +void WebPage::handleTwoFingerTapAtPoint(const WebCore::IntPoint& point, uint64_t callbackID)

Could we reuse getPositionInformation() here, but make an async version of it.  That might be too heavy weight, but passing enough information for a _WKActivatedElementInfo would be good.
Comment 4 Enrica Casucci 2016-03-09 11:15:46 PST
(In reply to comment #3)
> Comment on attachment 273364 [details]
I did notice the bot failures. I'll update UIKitSPI.h
> 
> > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:1321
> > +            if ([uiDelegate respondsToSelector:@selector(_webView:alternateActionForURL:)])
> > +                [uiDelegate _webView:webView alternateActionForURL:[NSURL _web_URLWithWTFString:string]];
> 
> Instead of passing a URL here, could we pass an _WKActivatedElementInfo (or
> subclass of it), and let the client make the decision?  That way, if someone
> wants to do something with images, or something else, they could.  It would
> also allow the client to show that something was happening, because the
> boundingRect would be available.
We are not computing the position information for this. We could in the future. For now I'd like to leave it as is.
> 
> > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:633
> > +void WebPage::handleTwoFingerTapAtPoint(const WebCore::IntPoint& point, uint64_t callbackID)
> 
> Could we reuse getPositionInformation() here, but make an async version of
> it.  That might be too heavy weight, but passing enough information for a
> _WKActivatedElementInfo would be good.
We already have requestPositionInformation that is async but doesn't use a completion block. I'd rather do this in a separate patch.
Comment 5 Enrica Casucci 2016-03-09 11:29:38 PST
Committed revision 197866.