Summary: | [Chromium, Mobile] Do not show disambiguation pop up in mobile sites | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | dfalcantara | ||||||||
Component: | WebKit Misc. | Assignee: | dfalcantara | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aelias, dglazkov, klobag, trchen, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
dfalcantara
2013-01-22 17:56:28 PST
Created attachment 184097 [details]
Patch
Comment on attachment 184097 [details] Patch Attachment 184097 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16039885 New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html Comment on attachment 184097 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184097&action=review > Source/WebKit/chromium/src/WebViewImpl.cpp:758 > + // Don't trigger a disambiguation popup on sites designed for mobile devices. > + // Instead, assume that the page has been designed with big enough buttons and links. > + ViewportArguments viewportArguments = mainFrameImpl()->frame()->document()->viewportArguments(); > + bool isLikelyMobileSite = !viewportArguments.userZoom || (viewportArguments.width == ViewportArguments::ValueDeviceWidth); Do we have other heuristics like this elsewhere? Should we use the same one consistently? If not, should we factor this code in a way that future code that wants a "is mobile site" heuristic can use it too? Also, we need a test. > Do we have other heuristics like this elsewhere? Should we use the same one consistently? If not, should we factor this code in a way that future code that wants a "is mobile site" heuristic can use it too?
>
> Also, we need a test.
I think I found the code you mentioned for the 300ms simulated click event delay Chromium-side, in ContentViewGestureHandler.java. It seems pretty specific, though: because our double tap action is to zoom in, we simply don't enforce the trigger the delay when there is a fixed page scale.
I'll dig around some more for other current possible use-cases, but I think factoring out the code is a good idea, regardless.
Created attachment 184346 [details]
Patch
Created attachment 184542 [details]
Patch
Alex: is this what you were going for? LGTM Ping? Comment on attachment 184542 [details]
Patch
Thanks. This is much better.
Comment on attachment 184542 [details] Patch Clearing flags on attachment: 184542 Committed r141019: <http://trac.webkit.org/changeset/141019> All reviewed patches have been landed. Closing bug. |