New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 675730 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

PDF viewer: pinch-zoom invokes native browser zoom when passive-listener-default is set

Project Member Reported by mcnee@chromium.org, Dec 19 2016

Issue description

Chrome Version: 57.0.2946.0
OS: Chrome OS

What steps will reproduce the problem?
(1) On a device with a touch screen, go to chrome://flags/#passive-listener-default
(2) Set the "Passive Event Listener Override" flag to "True (when unspecified)" and restart to have the change take effect
(3) Open a pdf (e.g. http://www.orimi.com/pdf-test.pdf)
(4) Pinch zoom the pdf

What is the expected result?
The content of the pdf should be zoomed and the controls should not (i.e. the same behaviour as when the override is not set).

What happens instead?
The controls are zoomed off the screen (i.e. the native browser zoom is invoked).

In the pdf viewer's touchstart listener, we call preventDefault on two finger touches to prevent native pinch-zoom. This listener needs to be non-passive.
 
Cc: puneetster@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16b6154685aff544f8870a8ca164d0181881c038

commit 16b6154685aff544f8870a8ca164d0181881c038
Author: mcnee <mcnee@chromium.org>
Date: Thu Dec 22 16:53:14 2016

PDF viewer: Specify that the touchstart listener not be passive.

In the PDF viewer's touchstart listener, we call preventDefault on
two finger touches to prevent native pinch-zoom. Hence, this listener
cannot be passive.

Currently, we do not specify whether our touch handlers are passive,
so if listeners are passive by default, the call to preventDefault
is ignored and native pinch-zoom is invoked.

We now specify the passive option when adding touch event listeners.

BUG= 675730 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2584393003
Cr-Commit-Position: refs/heads/master@{#440440}

[modify] https://crrev.com/16b6154685aff544f8870a8ca164d0181881c038/chrome/browser/resources/pdf/gesture_detector.js
[modify] https://crrev.com/16b6154685aff544f8870a8ca164d0181881c038/chrome/test/data/pdf/gesture_detector_test.js

Comment 3 by mcnee@chromium.org, Dec 22 2016

Status: Fixed (was: Started)
Labels: Merge-Request-56
Project Member

Comment 5 by sheriffbot@chromium.org, Jan 5 2017

Labels: -Merge-Request-56 Hotlist-Merge-Approved Merge-Approved-56
Your change meets the bar and is auto-approved for M56. Please go ahead and merge the CL manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 6 2017

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f588786d5f3fa1d4e4190111bd55f892b04b398a

commit f588786d5f3fa1d4e4190111bd55f892b04b398a
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Fri Jan 06 15:31:20 2017

PDF viewer: Specify that the touchstart listener not be passive.

In the PDF viewer's touchstart listener, we call preventDefault on
two finger touches to prevent native pinch-zoom. Hence, this listener
cannot be passive.

Currently, we do not specify whether our touch handlers are passive,
so if listeners are passive by default, the call to preventDefault
is ignored and native pinch-zoom is invoked.

We now specify the passive option when adding touch event listeners.

BUG= 675730 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2584393003
Cr-Commit-Position: refs/heads/master@{#440440}
(cherry picked from commit 16b6154685aff544f8870a8ca164d0181881c038)

Review-Url: https://codereview.chromium.org/2612773006 .
Cr-Commit-Position: refs/branch-heads/2924@{#686}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/f588786d5f3fa1d4e4190111bd55f892b04b398a/chrome/browser/resources/pdf/gesture_detector.js
[modify] https://crrev.com/f588786d5f3fa1d4e4190111bd55f892b04b398a/chrome/test/data/pdf/gesture_detector_test.js

Comment 7 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 8 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 9 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 11 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment