Bug 149968

Summary: Web pages with unscalable viewports shouldn't have a single tap delay
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebKit Misc.Assignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: 18runt88, benjamin, commit-queue, daes234, jonlee, rbyers, redux, simon.fraser, webkit-bug-importer, webkit
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: iPhone / iPad   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=153180
Bug Depends on:    
Bug Blocks: 122212, 150382    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Wenson Hsieh 2015-10-09 14:15:11 PDT
When a page has an unscalable viewport (for instance, user-scalable = no or minimum-scale = 1, maximum-scale = 1) we should not have to wait 350 ms for the double tap gesture recognizer to fire.
Comment 1 Wenson Hsieh 2015-10-09 15:15:32 PDT
<rdar://problem/23054453>
Comment 2 Alexey Proskuryakov 2015-10-10 01:02:29 PDT
Can we just get rid of unscalable web pages instead? These are the worst.
Comment 3 Wenson Hsieh 2015-10-10 15:33:09 PDT
Created attachment 262834 [details]
Patch
Comment 4 Wenson Hsieh 2015-10-11 09:59:24 PDT
Created attachment 262864 [details]
Patch
Comment 5 Simon Fraser (smfr) 2015-10-12 16:22:05 PDT
Comment on attachment 262864 [details]
Patch

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

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:2266
> +- (void)_setDoubleTapGesturesEnabled:(BOOL)enabled
> +{
> +    if (enabled && ![_doubleTapGestureRecognizer isEnabled]) {
> +        // The first tap recognized after re-enabling double tap gestures will not wait for the
> +        // second tap before committing. To fix this, we use a new double tap gesture recognizer.
> +        [self removeGestureRecognizer:_doubleTapGestureRecognizer.get()];
> +        [_doubleTapGestureRecognizer setDelegate:nil];
> +        [self _createAndConfigureDoubleTapGestureRecognizer];
> +    }
> +    [_doubleTapGestureRecognizer setEnabled:enabled];
> +}

Why can't we do this when we adjust the UIScrollView zoom scales? We know if it's zoomable there.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:252
> +    bool viewportIsUserScalable = userCanScaleViewport(m_viewportConfiguration);
> +    if (viewportWasUserScalable != viewportIsUserScalable)
> +        send(Messages::WebPageProxy::ViewportUserScalableDidChange(viewportIsUserScalable));

This should not be a new message (if it's needed at all); it should be in the transaction.
Comment 6 Wenson Hsieh 2015-10-13 21:54:57 PDT
Created attachment 263057 [details]
Patch
Comment 7 WebKit Commit Bot 2015-10-14 14:21:26 PDT
Comment on attachment 263057 [details]
Patch

Clearing flags on attachment: 263057

Committed r191072: <http://trac.webkit.org/changeset/191072>
Comment 8 WebKit Commit Bot 2015-10-14 14:21:30 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Patrick H. Lauke 2015-10-14 16:32:32 PDT
Great stuff and fast work there. Interesting to note that my proposal to tackle this has been sitting in limbo for 2 years https://bugs.webkit.org/show_bug.cgi?id=122212