Bug 155205

Summary: Add two finger tap on links
Product: WebKit Reporter: Enrica Casucci <enrica>
Component: WebKit2Assignee: Enrica Casucci <enrica>
Status: RESOLVED FIXED    
Severity: Normal CC: sam, thorton
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch sam: review+

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.