Issue metadata
Sign in to add a comment
|
onKeyup event is broken for PrintScreen key in the latest dev version
Reported by
shima...@gmail.com,
Nov 28 2016
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.3 Safari/537.36 Steps to reproduce the problem: 1. Access to the web page: http://www.broadband-xp.com/test/chrome/onkeyup.html 2. Press any key and you will see an alert with keycode. 3. Press PrintScreen key. What is the expected behavior? After pressing printscreen key, it should show "44". What went wrong? After pressing printscreen key, it does not show anything. Did this work before? Yes 56.0.2906.0 Chrome version: 56.0.2924.3 Channel: dev OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: Shockwave Flash 23.0 r0
,
Nov 28 2016
,
Nov 28 2016
,
Nov 29 2016
Tested the issue on chrome Stable #54.0.2840.99, Canary 57.0.2935.3 in Windows 10.0 and was able to reproduce the issue. This is a Non-Regression issue since seeing this from M30 #30.0.1549.0, Making the status to Untriaged so that the issue would get addressed. Note : Not able to reproduce the issue in MAC 10.11.6 and Linux Ubuntu 14.04. Thank you.
,
Nov 29 2016
@rbasuvula This is a regression issue. I do not know why your 54.0.2840.99 does not show an alert message "44". Now I am commenting from Windows 10 with Google Chrome 54.0.2840.99. Maybe my English is misleading. How to reproduce. 1. Go to http://www.broadband-xp.com/test/chrome/onkeyup.html with 54.0.2840.99 (stable) 2. Press "a" key. You will see an alert message "65", which is the keycode of "a". If you do not see any message, the window just loses focus. Click any point within the window to get the focus. 3. Click OK to close an alert message. 4. Press PrintScreen key. You will see an alert message "44", which is the keycode of "PrintScreen" key. 5. Then with the dev version go to http://www.broadband-xp.com/test/chrome/onkeyup.html 6. Press "a" key. You will see an alert message "65", which is the keycode of "a". If you do not see any message, the window just loses focus. Click any point within the window to get the focus. This is just fine. 7. Click OK to close an alert message. 8. Press PrintScreen key. You will see "No" alert message this time. The point is "onkeyup" event. onKeydown event is not triggered with PrintScreen key from the very beginning. I do not mention about it. Thank you in advance.
,
Nov 30 2016
@ shimax21 : Thanks for your update and clear steps.
,
Nov 30 2016
Tested in chrome stable #54.0.2840.99, Beta #55.0.2883.59, Dev #56.2924.10 and canary #56.0.2936.0 on Win 10.0 and was able to reproduce the issue on Dev and canary builds. Below are the Bisect Details: Bisect Info: ============= Good Build: 56.0.2914.0 (Revision - 430837) Bad Build: 56.0.2915.0 (Revision - 431137) Bisect URL: =========== You are probably looking for a change made after 430938 (known good), but no later than 430939 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/3fbe4d26c6c6920babf2a83b3741b4e94ce689ba..35d322e24f91a372ecdc0b152891e0635187a07e From the CL above, assigning the issue to the concern owner @ foolip : ------------------ Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner. Review-Url: https://codereview.chromium.org/2482853002
,
Dec 1 2016
Thank you for reporting this issue. I can confirm that https://codereview.chromium.org/2482853002 is the cause and that some changes like this were expected. I did not know that PrintScreen specifically would be affected. Note that before this change, just the keyup was fired, not the keydown or keypress events. I would argue this was a bug, if there's any good reason to suppress the first two events than the last should also be suppressed. http://www.broadband-xp.com/test/chrome/onkeyup.html is a test page, can you explain how you first came across this issue? Has it broken a real page? If there's a good reason to suppress keydown and keypress then I think this issue has to be WontFix, and otherwise I think this fix would be to allow all of the events. dtapuska@, would you happen to know why PrintScreen is suppressed in the first place?
,
Dec 1 2016
I am myself working on developments of many intranet sites in Japan that disable PrintScreen key or display warning when it is suppressed. So I came to know this bug. @ foolip : I am sorry that I was not able to understand all of your English. Before this change, to be sure onkeydown and onkeypress events were not fired for PrintScreen key. Only onkeyup event is fired. But now after this change, not only onkeydown and onkyepress but also onkeyup keyup were not fired for PrintScreen key. So there is no way any more to detect PrintScreen key in Google Chrome. That is the problem.
,
Dec 1 2016
No worries about the English, let's work this out :) Do you mean that you're trying to prevent the user from using the print screen key by calling event.preventDefault() or similar, or do the sites react somehow to it without preventing it?
,
Dec 1 2016
@foolip : That's right. But I can not explain about how it works in detail. It is just secret.
,
Dec 21 2016
@foolip : Since Christmas is coming, you must be busy now, but how are things going on? Is this difficult to fix this issue while addressing to the problem at the time of the full screen?
,
Jan 16 2017
Gentle Ping !! Could we get any update on this issue.
,
Jan 16 2017
This is on my list of things to take care of. I haven't looked into it yet but I trust it can be fixed one way or another for M57.
,
Jan 16 2017
I uploaded another test case in order to illustrate this problem from different perspectives. http://www.broadband-xp.com/test/chrome/onkeyup_dom.html In Google Chrome 55, as long as the window has a focus, PrintScreen key can always be detected. On the other hand, in Google Chrome 56 or above, once the window has lost the focus, getting back the focus is not enough. Instead a normal key such as "a" or "z" must be pressed in advance in order to detect PrintScreen key.
,
Jan 17 2017
,
Jan 17 2017
,
Jan 18 2017
,
Jan 19 2017
https://jsfiddle.net/w8pmbgtn/ logs keydown, keypress and keyup events. Using this, it looks like Firefox also only fires a keyup event. Although a bare keyup event is pretty strange, preserving the old behavior and matching Firefox is probably best. It looks as though the RawKeyDown and Char events actually never reach the code that I changed in https://codereview.chromium.org/2482853002, investigating now.
,
Jan 19 2017
Edge also only fires a keyup event.
,
Jan 19 2017
The culprit here is the RenderWidgetHostImpl::SuppressEventsUntilKeyDown() call from RenderWidgetHostViewAura::OnWindowFocused that happens on first load. Any bare keypress or keyup events will be dropped until there's a keydown event because of this.
,
Jan 19 2017
My plan now is to exclude PrintScreen specifically from the suppression in a first CL, and then try to remove SuppressEventsUntilKeyDown() entirely in a follow-up which is more risky but seems like a better fix. It was introduced in https://codereview.chromium.org/12221133 but seemingly isn't needed any more in some ad-hoc testing. To be continued tomorrow.
,
Jan 20 2017
Let's punt to M57, we're four days from cutting the first stable candidate for M56 and I don't think is severe for a merge to make sense. We're generally only accepting high severity security fixes / crashes at this point.
,
Jan 20 2017
#23, understood. OP, I'm sorry, but this means that the problem will be in Chrome 56. You reported this early on and it was confirmed to be a problem, but due to the wrong milestone label being applied I thought I could defer this for a while and still make it. Please accept my apologies for this situation.
,
Jan 20 2017
In fixing this, I've noticed that on Linux, event.keyCode is 42, while on Windows it is 44. This was tested with the same USB keyboard. In both cases, however, event.key and event.code are "PrintScreen", so there is some mapping to harmonize the two platforms. Mac not tested.
,
Jan 20 2017
Oops, on Linux, actually event.key is "Print", filed issue 683097 .
,
Jan 20 2017
Reviews: https://codereview.chromium.org/2641313002/ and https://codereview.chromium.org/2643243002 (will send to sadrul@ as it touches aura)
,
Jan 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e5fd75af8171718a84d387486cfba0ed225c6f5f commit e5fd75af8171718a84d387486cfba0ed225c6f5f Author: foolip <foolip@chromium.org> Date: Wed Jan 25 15:24:33 2017 Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown() This revert https://codereview.chromium.org/12221133 where the suppression on focus was originally added. No tests were added, but the manual test described now passes without this code. There is of course some chance that it is needed for some other case, so this CL should be revert at the first sign of trouble. The reason for this change is that the suppression on focus causes the PrintScreen KeyUp event to be dropped. BUG= 668969 Review-Url: https://codereview.chromium.org/2643243002 Cr-Commit-Position: refs/heads/master@{#446023} [modify] https://crrev.com/e5fd75af8171718a84d387486cfba0ed225c6f5f/content/browser/renderer_host/render_widget_host_impl.cc [modify] https://crrev.com/e5fd75af8171718a84d387486cfba0ed225c6f5f/content/browser/renderer_host/render_widget_host_impl.h [modify] https://crrev.com/e5fd75af8171718a84d387486cfba0ed225c6f5f/content/browser/renderer_host/render_widget_host_view_aura.cc
,
Jan 25 2017
,
Jan 26 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eef386e8b37005016c26e10e839e8984b98ad4da commit eef386e8b37005016c26e10e839e8984b98ad4da Author: Philip Jägenstedt <foolip@chromium.org> Date: Thu Jan 26 17:50:27 2017 Remove RenderWidgetHostImpl::SuppressEventsUntilKeyDown() This revert https://codereview.chromium.org/12221133 where the suppression on focus was originally added. No tests were added, but the manual test described now passes without this code. There is of course some chance that it is needed for some other case, so this CL should be revert at the first sign of trouble. The reason for this change is that the suppression on focus causes the PrintScreen KeyUp event to be dropped. BUG= 668969 Review-Url: https://codereview.chromium.org/2643243002 Cr-Commit-Position: refs/heads/master@{#446023} (cherry picked from commit e5fd75af8171718a84d387486cfba0ed225c6f5f) Review-Url: https://codereview.chromium.org/2652893012 . Cr-Commit-Position: refs/branch-heads/2987@{#109} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/eef386e8b37005016c26e10e839e8984b98ad4da/content/browser/renderer_host/render_widget_host_impl.cc [modify] https://crrev.com/eef386e8b37005016c26e10e839e8984b98ad4da/content/browser/renderer_host/render_widget_host_impl.h [modify] https://crrev.com/eef386e8b37005016c26e10e839e8984b98ad4da/content/browser/renderer_host/render_widget_host_view_aura.cc
,
Jan 26 2017
,
Jan 29 2017
I also confirmed that this bug had been fixed in the latest 57.0.2987.13(Official Build)dev (64bit). Thank you so much for all of those who have contributed to the fix.
,
Jan 31 2017
Issue 686685 has been merged into this issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ajha@chromium.org
, Nov 28 2016