| Issue 360602 | Click delay initially left enabled on width=device-width page | ||||||||||||||||||||||||
| Starred by 4 users | Project Member Reported by rbyers@chromium.org, Apr 7, 2014 | Back to list | |||||||||||||||||||||||
Sign in to add a comment
|
Chrome Version : 34.0.1847.114 URLs (if applicable) : http://quirksmode.org/m/tests/clickdelay.html What steps will reproduce the problem? 1. On Android device (eg. Nexus 7) 2. Open http://quirksmode.org/m/tests/clickdelay.html 3. Tap a few times - verify you get ~250ms delay 4. Now tap on the "with width=device-width" link (navigates to clickdelay1.html) 5. Now tap a few times What is the expected result? Expect consistently small values (<10ms) What happens instead of that? Often (but not always) the very first value is ~250ms. Please provide any additional information below. Attach a screenshot if possible. I think there must be some sort of race in disabling the click delay on an existing renderer. Here's related things I've tested: - on nexus 5: looks to behave about the same - Current chrome beta and dev (m35) channels: same - Using a different test page (http://jsbin.com/cukeg/1/quiet): about the same - With double-taps instead of tap (watch for zoom instead of timing which could be influenced by other things): - Close all taps, restart browser, and open the test page fresh: doesn't repro - Interact with the page at all (eg. pinch, or attempt to scroll down) before the first tap: seems to avoid the problem So I'm guessing this is a race with getting the new double-tab-enabled bit as part of the commit from the compositor. Do we need a setNeedsCommit when it changes? I'm hoping this explains the "chrome weirdness" ppk blogged about here: http://www.quirksmode.org/blog/archives/2014/04/suppressing_the.html. I haven't been able to reproduce the exact behavior he describes.
,
Apr 8, 2014
------------------------------------------------------------------ r262312 | jdduke@chromium.org | 2014-04-08T04:49:50.258720Z Changed paths: M http://src.chromium.org/viewvc/chrome/trunk/src/content/public/android/java/src/org/chromium/content/browser/RenderCoordinates.java?r1=262312&r2=262311&pathrev=262312 M http://src.chromium.org/viewvc/chrome/trunk/src/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java?r1=262312&r2=262311&pathrev=262312 [Android] Use latest page scale factor when computing content width Previously, ContentViewCore clamped the effective content bounds from the compositor frame metadata using the *previous* frame's page scale factor. Instead, clamp the content bounds using the *current* frame's page scale factor, and make the content viewport comparison more robust. This fixes an issue where the content size after page load was being improperly calculated, preventing mobile sites (e.g., width=device-width) from benefiting from the disabled tap delay. BUG= 360602 Review URL: https://codereview.chromium.org/224723019 -----------------------------------------------------------------
,
Apr 8, 2014
,
Apr 8, 2014
OK so aelias@ provided me with a N7 V1, and I can definitely repro the consistent width=device-width failure. Turns out, it very much is floating point error, and the epsilon from r262312 is actually too small by a good margin. I'll increase the epsilon to accommodate this, and we can cherry pick both patches to M35.
,
Apr 8, 2014
Both https://codereview.chromium.org/224723019 and the pending https://codereview.chromium.org/228613006/ are low-risk, high-reward fixes for several tap-delay related issues. Cherry-picking to M35 will make the world a better place, sooner.
,
Apr 8, 2014
Awesome, thanks Jared! Expanding this bug to cover both issues. So in addition to the repro steps at the beginning of the bug: 1) Use a device with a non-integer device-scale-factor like a 2012 Nexus 7 (window.devicePixelRatio=1.33125... as opposed to the 2013's value of 2) 2) open http://quirksmode.org/m/tests/clickdelay1.html 3) tap - always see delay even though you shouldn't
,
Apr 8, 2014
------------------------------------------------------------------ r262479 | jdduke@chromium.org | 2014-04-08T19:41:16.525597Z Changed paths: M http://src.chromium.org/viewvc/chrome/trunk/src/content/public/android/java/src/org/chromium/content/browser/RenderCoordinates.java?r1=262479&r2=262478&pathrev=262479 [Android] Increase the mobile viewport epsilon threshold On an N7 V1, the delta between computed content and viewport widths when width=device-width is ~0.065. Increase the |hasMobileViewport()| query epsilon when comparing these values to appropriately classify mobile sites. BUG= 360602 NOTRY=true Review URL: https://codereview.chromium.org/228613006 -----------------------------------------------------------------
,
Apr 9, 2014
can u pls confirm on tot before u merge? ty :)
,
Apr 9, 2014
Yup, confirmed, thanks!
,
Apr 9, 2014
,
Apr 9, 2014
------------------------------------------------------------------ r262814 | jdduke@chromium.org | 2014-04-09T21:12:57.776102Z Changed paths: M http://src.chromium.org/viewvc/chrome/branches/1916/src/content/public/android/java/src/org/chromium/content/browser/RenderCoordinates.java?r1=262814&r2=262813&pathrev=262814 M http://src.chromium.org/viewvc/chrome/branches/1916/src/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java?r1=262814&r2=262813&pathrev=262814 Merge 262312 "[Android] Use latest page scale factor when comput..." > [Android] Use latest page scale factor when computing content width > > Previously, ContentViewCore clamped the effective content bounds from the > compositor frame metadata using the *previous* frame's page scale factor. > Instead, clamp the content bounds using the *current* frame's page scale > factor, and make the content viewport comparison more robust. This fixes an > issue where the content size after page load was being improperly calculated, > preventing mobile sites (e.g., width=device-width) from benefiting from the > disabled tap delay. > > BUG= 360602 > > Review URL: https://codereview.chromium.org/224723019 TBR=jdduke@chromium.org Review URL: https://codereview.chromium.org/231933002 -----------------------------------------------------------------
,
Apr 9, 2014
------------------------------------------------------------------ r262817 | jdduke@chromium.org | 2014-04-09T21:13:54.319161Z Changed paths: M http://src.chromium.org/viewvc/chrome/branches/1916/src/content/public/android/java/src/org/chromium/content/browser/RenderCoordinates.java?r1=262817&r2=262816&pathrev=262817 Merge 262479 "[Android] Increase the mobile viewport epsilon thr..." > [Android] Increase the mobile viewport epsilon threshold > > On an N7 V1, the delta between computed content and viewport widths when > width=device-width is ~0.065. Increase the |hasMobileViewport()| query epsilon > when comparing these values to appropriately classify mobile sites. > > BUG= 360602 > NOTRY=true > > Review URL: https://codereview.chromium.org/228613006 TBR=jdduke@chromium.org Review URL: https://codereview.chromium.org/231653003 ----------------------------------------------------------------- |
||||||||||||||||||||||||
| ► Sign in to add a comment | |||||||||||||||||||||||||
Status: Started