New issue
Advanced search Search tips

Issue 725630 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Update testPreventNativePinchZoom test to store listener options properly

Project Member Reported by mcnee@chromium.org, May 23 2017

Issue description

In gesture_detector_test.js, the |listenerOptions| map for the testPreventNativePinchZoom test is just keyed on the
listener. This may cause the test to fail if we add the same listener multiple times (for separate events) with different options.

We should update the test to not require that the listeners be referentially not equal.
 

Comment 1 by mcnee@chromium.org, May 23 2017

This was noticed in https://codereview.chromium.org/2886943006/
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 26 2017

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

commit 7c7a90d056f2529da672d20bd314235bbedd7e7b
Author: Kevin McNee <mcnee@chromium.org>
Date: Thu Oct 26 21:31:55 2017

PDF viewer: Store listener options properly in gesture_detector_test.js

We currently have a map for listener options that is just keyed on the
listener. So if a listener is added multiple times (for separate events)
the options would be overwritten.

The tests no longer require that the listeners be referentially not
equal. We also refactor our listener setup in GestureDetector which
previously would have caused the tests to incorrectly fail.

Bug:  725630 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I23dad121ccd3e896b714bd0fc0ab100f1928c0b1
Reviewed-on: https://chromium-review.googlesource.com/731248
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511962}
[modify] https://crrev.com/7c7a90d056f2529da672d20bd314235bbedd7e7b/chrome/browser/resources/pdf/gesture_detector.js
[modify] https://crrev.com/7c7a90d056f2529da672d20bd314235bbedd7e7b/chrome/test/data/pdf/gesture_detector_test.js

Comment 3 by mcnee@chromium.org, Oct 27 2017

Status: Fixed (was: Assigned)

Sign in to add a comment