Bug 150248 – Extend fast-clicking behavior to trigger on elements that have negligible zoom

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Wenson Hsieh]

Created attachment 263313 [details]
Test page for zoom-dependent fast clicking

We will want websites with scalable viewports to also benefit from fast-click behavior when applicable. One idea is to use the double-tap zoom factor as a heuristic to determine whether an element should quickly fire a click when tapped or wait for the double tap gesture to fail. If double tapping on an element will not cause the viewport to zoom very much, we should make single taps on it trigger fast clicking, since double-tapping will offer little value.

[Wenson Hsieh]

In regards to the test page: double tapping the top button should trigger zoom; double tapping the bottom button should fire clicks, since double-tap to zoom won't do much for us there anyways.

[Wenson Hsieh]

<rdar://problem/23140069>

[Wenson Hsieh]

Created attachment 263340 [details]
work in progress v1

[WebKit Commit Bot]

Attachment 263340 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:741:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:772:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Wenson Hsieh]

Created attachment 263412 [details]
Patch

[WebKit Commit Bot]

Attachment 263412 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:834:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:877:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 263412 [details]
Patch

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

> Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:940
> +void WebPageProxy::disableDoubleTapGesturesUntilTapIsFinishedIfNecessary(uint64_t requestID, bool allowsDoubleTapZoom, const WebCore::FloatRect& targetRect, bool isReplacedElement, double minimumScale, double maximumScale)

The "IfNecessary" is a big vague. Does that just refer to the potential zoom level change?

> Source/WebKit2/WebProcess/WebPage/ViewGestureGeometryCollector.cpp:104
> +void ViewGestureGeometryCollector::computeZoomInformationForNode(Node* node, FloatPoint& origin, FloatRect& renderRect, bool& isReplaced, double& viewportMinimumScale, double& viewportMaximumScale)

Node&

[Wenson Hsieh]

Comment on attachment 263412 [details]
Patch

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

Thanks for the review!

>> Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm:940
>> +void WebPageProxy::disableDoubleTapGesturesUntilTapIsFinishedIfNecessary(uint64_t requestID, bool allowsDoubleTapZoom, const WebCore::FloatRect& targetRect, bool isReplacedElement, double minimumScale, double maximumScale)
> 
> The "IfNecessary" is a big vague. Does that just refer to the potential zoom level change?

Yes -- we could potentially bail from disabling double tap gestures if the expected change in zoom level is too much. However, I hesitated when naming it something more specific like "IfZoomingIsNegligible" since the touch-action: manipulation patch will also use this codepath, and in that case a significant amount of zoom level change is overridden by having touch-action: manipulation on the element, so I thought "IfNecessary" would better capture both cases.

>> Source/WebKit2/WebProcess/WebPage/ViewGestureGeometryCollector.cpp:104
>> +void ViewGestureGeometryCollector::computeZoomInformationForNode(Node* node, FloatPoint& origin, FloatRect& renderRect, bool& isReplaced, double& viewportMinimumScale, double& viewportMaximumScale)
> 
> Node&

Got it, fixed. Since we never expect node to be null here, the reference makes a lot more sense.

[Wenson Hsieh]

Committed r191309: <http://trac.webkit.org/changeset/191309>