Issue metadata
Sign in to add a comment
|
Erroneous browser page zoom ignoring touch-action:none
Reported by
sebp.mue...@gmail.com,
Nov 3 2016
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36 Steps to reproduce the problem: 1. Use the current Beta or Canary version on a touch enabled device (I tested on Windows with a touch screen) 2. Go to http://live.yworks.com/yfiles-for-html/1.3/demos/graphviewer/demo.yfiles.graph.viewer/index.html and wait for the page to load. 3. Try pinch-zooming the graph in the center, first slowly, then quicker What is the expected behavior? Only the graph should zoom. The pointer events are captured, interpreted, and handled by the Javascript application and the browser should not ever zoom the whole page for this single page application. What went wrong? If you pinch slowly, everything works as expected. If you pinch quicker (and more events are generated in rapid succession) at some point suddenly the browser interprets and handles the pinch gesture itself and initiates a page zoom, not delivering the events to the javascript anymore. This results in a broken user experience where suddenly the UI gets zoomed, instead of the content. Did this work before? Yes 54.0.2840.87 m Does this work in other browsers? Yes Chrome version: 56.0.2907.0 Channel: dev OS Version: 10.0 Flash Version: I suspect this has to do with the new pointer events API. The application uses pointer events API if available and works flawlessly using other UAs that implement that API. A timeline recording shows that all is fine if the events don't come too quickly. There is a "long frame" when suddenly the browser pinch zoom kicks in and the application does not seem to get the chance to handle the pointer events any longer. There is also a meta tag set on the application (width=device-width, initial-scale=1.0, user-scalable=no) and a "touch-action:none" css class on the element in question that should prevent this from ever happening and all code paths in the application that are involved should properly handle all events, still, when quickly pinching with two fingers, suddenly the page is zoomed as a whole. Also sometimes there is a warning on the console stating that there are two pointer events with the same timestamp, maybe the app only gets to "handle" one of them and the second one does not get "defaultprevented" and then the zoom kicks in? This is a severe regression with the current beta and canary versions that breaks existing pages that previously where working with the older touch API. Please don't release this code on stable!
,
Nov 3 2016
This is clearly an Windows-only issue. Doesn't repro in CrOS. sebp.mueller@gmail.com: I couldn't trigger the warning (events with same time-stamp) you mentioned. Could you please copy-paste the exact error message?
,
Nov 4 2016
Here is the warnings that I see - they only show when I open the time-line view. This is probably code in the timeline viewer that tries to sort the events for displaying: "Two touches at the same time? 2797110.19 vs 2797161.887" I found that when the problem appears I get additional touchstart/touchdown events in the timeline even though I still have only two fingers on the screen. It seems that these additional events are then handled by Chrome's internal pinch logic. I will see if I can create a smaller repro. How *do* you disable pinch zoom altogether for a page or element? shouldn't user-scalable=no and touch-action:none suffice?
,
Nov 4 2016
Here is a drastically reduced simple test-case that exhibits the bad behavior, too. This code is not very busy at all and simply prevents the default operation and does one method call. So my "long-frame theory" is probably wrong. Steps to reproduce: Open the file and do a pinch-zooming gesture on the blue rectangle. *Nothing* should happen except for the active pointer count in the rectangle being shown. However after some pinching the page gets zoomed, too.
,
Nov 4 2016
If you move touch-action: none to <html> or <body>, does it still reproduce?
,
Nov 4 2016
Yes, in a way this is an even better repro that does not even require Javascript: Putting "touch action none" on the body does not make a difference, but putting it on html prevents the pinch zoom from happening if you slowly pinch the page (the part that is unaffected by the event handling), but as soon as "the bug kicks in" the page is still pinch zoomed, no matter whether this is on the page or the test rectangle. So with the new API in place I don't see how you can ever disable pinch zoom. Neither using the meta-tag, nor using the css style on the element, body, or html, nor by capturing and preventing the default on all pointer events.
,
Nov 4 2016
And some more annoying questions :) -
1. Does * { touch-action: none; } help?
2. Does removing any JavaScript still reproduce the issue (along with 1)?
,
Nov 8 2016
,
Nov 21 2016
Kind request for an update of the status of this bug. If you have *any* workaround for this regression, please post it, otherwise can we make this a blocker so that it doesn't get released to stable? This bug will break all non-trivial applications that use the touch API, won't it? You can't consume events/prevent default on them, they will always result in scrolling or zooming of the page... This should break a lot of games and sophisticated software, I believe...
,
Nov 21 2016
I did a quick test early last week, no clue what's happening here. I initially suspected a low (device-drive) level crack, couldn't confirm. Unfortunately I didn't get enough time to deep dive. Will do today/tomorrow.
,
Nov 22 2016
Here is an easier, more predictable way to repro where speed of pinching doesn't matter: 1. Open the demo in #c8 above (also available here: http://output.jsbin.com/towaxod) 2. Do a pinch zoom-out (with fingers getting closer). 3. Continue to a pinch zoom-in (with fingers getting farther). No matter how fast or slow the fingers moves, the page starts to zoom in Step 3 /after/ the fingers move a certain distance.
,
Nov 23 2016
- Not a Windows only issue, could easily repro in CrOS following #c12. - M54 works as expected in both Win & CrOS. - Root cause: GesturePinch* event sequence received at TouchActionFilter seems broken. While in Step 3 of #c12 above (near the beginning), TouchActionFilter receives a GesturePinchEnd & then a GesturePinchBegin. So we have two pinch sequences (while a single gesture scroll sequence as expected). Here is a sample event sequence: GestureTapDown GestureTapCancel GestureScrollBegin GestureScrollUpdate+ GesturePinchBegin (GestureScrollUpdate & GesturePinchUpdate)+ GestureScrollUpdate GesturePinchEnd GestureScrollUpdate+ GesturePinchBegin (GestureScrollUpdate & GesturePinchUpdate)+ GesturePinchEnd GestureScrollEnd
,
Nov 23 2016
,
Nov 23 2016
This is not related to PointerEvents: switching the flag doesn't change the outcome.
,
Nov 23 2016
Bumping up the priority. This should be merged to even M55, right?
,
Nov 23 2016
Yeah, we should try to get this merged before the stable release.
,
Nov 23 2016
,
Nov 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8dc459398704fe6df59e1be0655399085454449 commit b8dc459398704fe6df59e1be0655399085454449 Author: mustaq <mustaq@chromium.org> Date: Wed Nov 23 21:13:16 2016 Removed resetting of pinch-gesture filter state on pinch end. We used to reset the TouchActionFilter state |drop_pinch_gesture_events_| at a pinch end assuming that there is a single pinch-sequence within a gesture-scroll sequence. This logically correct assumption turned out to be wrong in practice: a pinch-zoom-out-then-zoom-in actually sends two consecutive pinch-begin/end sequences between a gesture-scroll-begin/end sequence. This CL defers the resetting of |drop_pinch_gesture_events_| to bracketing gesture-scroll-end to avoid wrong filtering during the second pinch-begin/end sequence. BUG= 662047 Review-Url: https://codereview.chromium.org/2526433003 Cr-Commit-Position: refs/heads/master@{#434248} [modify] https://crrev.com/b8dc459398704fe6df59e1be0655399085454449/content/browser/renderer_host/input/touch_action_filter.cc [modify] https://crrev.com/b8dc459398704fe6df59e1be0655399085454449/content/browser/renderer_host/input/touch_action_filter_unittest.cc
,
Nov 23 2016
,
Nov 24 2016
I don't think this is the correct long term fix. We shouldn't be sending a pinch end in this case. Did you determine why we're sending a pinch end?
,
Nov 24 2016
Yes it's an ad-hoc fix. Merging to M55 was my priority, so didn't look for a deeper change. I have added a TODO in the code, will file a bug soon.
,
Nov 24 2016
Before we approve merge to M55, could you please make sure this change is well baked/verified in Canary and safe to merge to M55?
,
Nov 24 2016
Will verify once a Canary with the change is available.
,
Nov 24 2016
Your change meets the bar and is auto-approved for M55 (branch: 2883)
,
Nov 25 2016
Verified the fix in win canary (57.0.2931.0): both the minimal repro and the actual site works correctly. Will merge to M55 now.
,
Nov 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b51fc4e484f4d46830b40122b04119e0edda708d commit b51fc4e484f4d46830b40122b04119e0edda708d Author: Mustaq Ahmed <mustaq@google.com> Date: Fri Nov 25 16:16:17 2016 Removed resetting of pinch-gesture filter state on pinch end. We used to reset the TouchActionFilter state |drop_pinch_gesture_events_| at a pinch end assuming that there is a single pinch-sequence within a gesture-scroll sequence. This logically correct assumption turned out to be wrong in practice: a pinch-zoom-out-then-zoom-in actually sends two consecutive pinch-begin/end sequences between a gesture-scroll-begin/end sequence. This CL defers the resetting of |drop_pinch_gesture_events_| to bracketing gesture-scroll-end to avoid wrong filtering during the second pinch-begin/end sequence. BUG= 662047 Review-Url: https://codereview.chromium.org/2526433003 Cr-Commit-Position: refs/heads/master@{#434248} (cherry picked from commit b8dc459398704fe6df59e1be0655399085454449) Review URL: https://codereview.chromium.org/2530213002 . Cr-Commit-Position: refs/branch-heads/2883@{#663} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/b51fc4e484f4d46830b40122b04119e0edda708d/content/browser/renderer_host/input/touch_action_filter.cc [modify] https://crrev.com/b51fc4e484f4d46830b40122b04119e0edda708d/content/browser/renderer_host/input/touch_action_filter_unittest.cc
,
Nov 29 2016
Verified this issue on Windows-10 (Dell Precisiom M-3800) using chrome latest M55-55.0.2883.70 by following test case provided in the comment #12. Observed no zooming while pinching in and out of the screen. Hence adding TE-Verified label. Note: Attaching screen-cast of the previous and current behavior of the issue for reference.
,
Nov 29 2016
I can confirm that the bug is fixed for my original usecase in Version 57.0.2935.3 canary (64-bit) on Windows. Thanks for fixing.
,
Feb 16 2017
,
Mar 20 2017
Issue 701962 has been merged into this issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by majidvp@chromium.org
, Nov 3 2016Labels: M-56 Hotlist-Input-Dev
Owner: mustaq@chromium.org
Status: Assigned (was: Unconfirmed)