| Summary: | Extend fast-clicking behavior to trigger on elements that have negligible zoom | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
| Component: | WebKit Misc. | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | benjamin, commit-queue, ggaren, jonlee, redux, sam, simon.fraser, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 122212 | ||||||||||
| Attachments: |
|
||||||||||
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. Created attachment 263340 [details]
work in progress v1
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.
Created attachment 263412 [details]
Patch
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.
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& 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. Committed r191309: <http://trac.webkit.org/changeset/191309> |
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.