Issue metadata
Sign in to add a comment
|
Regression : Page does not get zoomed on pressing 'cmd ++ '
Reported by
pranjali...@etouch.net,
Jul 26
|
||||||||||||||||||||||
Issue descriptionChrome version : 70.0.3502.0 (Official Build) bd32163382ec789e29eeceaf695587d76d77eb36-refs/branch-heads/3502@{#1} (64 bit) OS: Mac(10.13.1,10.14) OS What steps will reproduce the problem? 1.Launch chrome and open NTP. 2. Now press 'cmd + +' and observe. Actual : Nothing happens on pressing 'cmd + +' Expected: Page should get zoom in and zoom bubble should be seen on pressing 'cmd ++' This is a regression issue broken in ‘M-70’ and below is bisect info. Good build: 70.0.3501.0 Bad build: 70.0.3502.0 You are probably looking for a change made after 577665 (known good), but no later than 577666 (first known bad). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/61c2840f00243135c7f22504ba0d8147db68b856..124f80f9c6f0935ed1369617346443d4321427bf Suspecting : https://chromium.googlesource.com/chromium/src/+/124f80f9c6f0935ed1369617346443d4321427bf @rsesek: Could you please check whether this is caused with respect to your change, if not please help to reassigning it to the right owner. Note:Issue is specific to Mac(19.13.1,10.14) and not seen on (10.12.6,10.13.6),Win(7,8,8.1,10) and Linux(14.04 LTS) Thank You!
,
Jul 26
,
Jul 26
I am unable to reproduce this. Can you please take as screenshot of the contents of the View menu in the menubar?
,
Jul 27
Update: Rechecked the above issue on latest canary build #70.0.3504.0 and issue is reproducible without enabling any flag. Kindly refer attached screencast and screenshot. Note:Issue is only for cmd ++ and not for cmd --.
,
Jul 31
I can confirm this with a built-in Mac keyboard but not with an external one. I think this is because the key equivalent is getting picked up as Cmd+Shift+Plus. But the plus character is inherently shifted, so the shift modifier is incorrect. GetKeyEquivalentAndModifierMaskFromAccelerator() calls MacKeyCodeForWindowsKeyCode(), which knows how to translate these special "shifted" characters, but it doesn't mask out NSEventModifierFlagShift for the characters that are inherently shifted. Erik: Do you have any thoughts on how to handle this? Maybe MacKeyCodeForWindowsKeyCode() needs an out shifted_flags parameter to mask off shift for special characters?
,
Jul 31
rsesek: do you have repro steps? I still can't repro on external or internal keyboard.
,
Jul 31
Interesting. I'm reproing on Chrome 70.0.3508.0 with a MacBook Pro 15-inch 2016 (MacBookPro13,3). Cmd+Plus does not zoom but Cmd+Shift+Plus does.
,
Jul 31
I cannot repro with 3508 Canary, MBP 15-inch 2017 [MacBookPro14,3].
,
Aug 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a6b5d320d944a405d2bba0dee46aa71d97c592c commit 9a6b5d320d944a405d2bba0dee46aa71d97c592c Author: Robert Sesek <rsesek@chromium.org> Date: Fri Aug 03 03:08:35 2018 [Mac] Fix Cmd+Plus to zoom keyboard shortcut. GetKeyEquivalentAndModifierMaskFromAccelerator() computes the key equivalent for menu items. If the key equivalent is inherently shifted (e.g. + and }), do not include Shift in the modifier flags, since it is already encoded in the equivalent. Including it in the flags results in Shift being required in addition to the key. Bug: 867788 Test: Cmd+Plus zooms the page, as well as Cmd+Shift+Plus. Change-Id: I3536d7652804f77a14b51197cdf5f49d6defc77d Reviewed-on: https://chromium-review.googlesource.com/1157224 Reviewed-by: Erik Chen <erikchen@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Robert Sesek <rsesek@chromium.org> Cr-Commit-Position: refs/heads/master@{#580427} [modify] https://crrev.com/9a6b5d320d944a405d2bba0dee46aa71d97c592c/ui/base/BUILD.gn [modify] https://crrev.com/9a6b5d320d944a405d2bba0dee46aa71d97c592c/ui/base/accelerators/platform_accelerator_cocoa.mm [add] https://crrev.com/9a6b5d320d944a405d2bba0dee46aa71d97c592c/ui/base/accelerators/platform_accelerator_cocoa_unittest.mm
,
Aug 3
,
Aug 3
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by pranjali...@etouch.net
, Jul 26