Issue metadata
Sign in to add a comment
|
Regression: Unable to close Print preview overlay by pressing 'Esc' key on Mac
Reported by
vineetha...@etouch.net,
Oct 1
|
||||||||||||||||||||||
Issue descriptionChrome Version : 71.0.3567.0 (Official Build) Revision 7c7bc73fc5f252e28f2308c81de4760e49990542-refs/branch-heads/3567@{#1} (32/64 bit) OS :Mac(10.12.6, 10.13.1, 10.13.6, 10.14.1) Steps to reproduce: 1. Launch chrome press 'Cmd+P' to open Print preview overlay. 2. Click anywhere on the preview section (i.e. RHS section) of print preview overlay. 3. Now, Press 'Esc' key and observe. Actual Result : Print preview overlay does not get closed on pressing 'Esc' key . Expected Result: Print preview overlay should get closed on pressing 'Esc' key This is a regression issue broken in ‘M-71’ and providing the bisect info below: Good Build : 71.0.3551.3 (Revision : 590851) Bad Build : 71.0.3552.2 (Revision : 591210) CHANGE-LOG URL: https://chromium.googlesource.com/chromium/src/+log/71.0.3551.0..71.0.3552.0?pretty=fuller&n=10000 Suspecting : r591109 ? @rbpotter : Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Note : 1) Issue is specific to Mac OS. 2) Providing suspect through 'Change-Log' because unable to perform bisect using 'per-revision' and 'chromium bisect' script. 3) Tried performing 'per revision' bisect on multiple Mac machines but unable to perform the same since getting following error: - Error message on Mac OS:[Errno 2] No such file or directory error message 4) Unable to perform 'chromium bisect' script as issue is not reproducible on chromium builds. 5) Observe that issue is not reproducible when focus is on LHS section of print preview overlay. Kindly refer the attached screen-cast. Thank you..!
,
Oct 3
Update w.r.t comment #1: Rechecked the above issue on Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14.1) OS with Canary version #69.0.3460.0 by enabling 'Enable new Print Preview UI' flag under chrome://flags and observed that , yes, the above issue is reproducible and exists from M69. Kindly refer the attached screen cast. Thank you!
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/74c0b48deaf648676bb7b4ee5238b7a2d37825db commit 74c0b48deaf648676bb7b4ee5238b7a2d37825db Author: rbpotter <rbpotter@chromium.org> Date: Thu Oct 04 01:24:02 2018 PDF Viewer: Forward code from key events Currently the pdf viewer forwards the keyCode property from key events. However, this property is deprecated, see: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode Add the non-deprecated code property to the list of properties to serialize. Note: this caused a problem with key handling in Print Preview, when Print Preview was updated from using keyCode to code to identify the escape key. Bug: 890724 Change-Id: I259dd36e344f8a5095487b71df8e96d85e3af47b Reviewed-on: https://chromium-review.googlesource.com/c/1256111 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/heads/master@{#596466} [modify] https://crrev.com/74c0b48deaf648676bb7b4ee5238b7a2d37825db/chrome/browser/resources/pdf/pdf_scripting_api.js [modify] https://crrev.com/74c0b48deaf648676bb7b4ee5238b7a2d37825db/chrome/test/data/pdf/basic_plugin_test.js
,
Oct 4
,
Oct 6
Since this change is very small and the bug impacts the new UI, which is currently rolling out in 69, it would be nice to merge this to M70 if possible, but we need to verify on Canary first. Reporter - can you test? Also cc-ing thestig@ who may be able to test this on Mac also.
,
Oct 8
Update : Rechecked the above issue on Mac(10.12.6 , 10.13.1 , 10.13.6, 10.14.1)OS with latestCanary version #71.0.3573.0 and the issue is fixed. Kindly refer the attached screen cast.
,
Oct 9
Requesting a merge to M-70 for the change in comment 3, which has been verified in Canary (see comment 6). This is a very small, safe change (2 lines of code + 2 lines of test modifications) that will ensure the user can always close the print dialog with the escape key on Mac, even if the preview area is focused. The change is also verified by the automated test modification in the same CL.
,
Oct 9
This bug requires manual review: We are only 6 days from stable. Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 9
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0e09aca901e8793da9b6a7fd2406c943354625ed Commit: 0e09aca901e8793da9b6a7fd2406c943354625ed Author: rbpotter@chromium.org Commiter: rbpotter@chromium.org Date: 2018-10-09 18:17:58 +0000 UTC PDF Viewer: Forward code from key events (M70) Currently the pdf viewer forwards the keyCode property from key events. However, this property is deprecated, see: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode Add the non-deprecated code property to the list of properties to serialize. Note: this caused a problem with key handling in Print Preview, when Print Preview was updated from using keyCode to code to identify the escape key. Bug: 890724 Change-Id: I259dd36e344f8a5095487b71df8e96d85e3af47b Reviewed-on: https://chromium-review.googlesource.com/c/1256111 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#596466}(cherry picked from commit 74c0b48deaf648676bb7b4ee5238b7a2d37825db) Reviewed-on: https://chromium-review.googlesource.com/c/1271576 Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#923} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0e09aca901e8793da9b6a7fd2406c943354625ed commit 0e09aca901e8793da9b6a7fd2406c943354625ed Author: rbpotter <rbpotter@chromium.org> Date: Tue Oct 09 18:17:58 2018 PDF Viewer: Forward code from key events (M70) Currently the pdf viewer forwards the keyCode property from key events. However, this property is deprecated, see: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode Add the non-deprecated code property to the list of properties to serialize. Note: this caused a problem with key handling in Print Preview, when Print Preview was updated from using keyCode to code to identify the escape key. Bug: 890724 Change-Id: I259dd36e344f8a5095487b71df8e96d85e3af47b Reviewed-on: https://chromium-review.googlesource.com/c/1256111 Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org> Commit-Queue: Rebekah Potter <rbpotter@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#596466}(cherry picked from commit 74c0b48deaf648676bb7b4ee5238b7a2d37825db) Reviewed-on: https://chromium-review.googlesource.com/c/1271576 Reviewed-by: Rebekah Potter <rbpotter@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#923} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/0e09aca901e8793da9b6a7fd2406c943354625ed/chrome/browser/resources/pdf/pdf_scripting_api.js [modify] https://crrev.com/0e09aca901e8793da9b6a7fd2406c943354625ed/chrome/test/data/pdf/basic_plugin_test.js
,
Oct 11
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rbpotter@chromium.org
, Oct 1Bisect is most likely incorrect. The "Expected" video is using the old UI (non-Polymer), based on the appearance of the "More settings" link. Based on the cause, this issue has probably been in the Polymerized print preview UI since the beginning ("New Print Preview" flag). The flag was turned on by default on ToT in the bisect range, which is what caused it to appear to regress in this range. If you have a Mac, you should be able to confirm by testing M69 or M70 with the "New Print Preview" flag enabled; I expect the bug will reproduce.