Closed
Bug 582052
Opened 15 years ago
Closed 14 years ago
certain Cmd and Cmd+Shift key combinations dispatch two keypress events
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: myk, Assigned: jaas)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
580 bytes,
text/html
|
Details |
When you press Cmd+; (semi-colon), Cmd+Shift+A, and certain other key combinations (I haven't made an exhaustive survey, but Cmd+Shift+U, Cmd+Shift+x also exhibit this behavior) in a Minefield nightly build on Mac OS X, two keypress events get dispatched instead of the customary one.
Calling event.preventDefault on the first event prevents this from happening. And it doesn't appear to happen for keys that have a default behavior (f.e. Cmd+A, Cmd+X, Cmd+Shift+F).
The attached simple testcase demonstrates the problem. Load it, then press one of the aforementioned key combinations. The testcase adds the word "keypress" to its DOM each time it handles a keypress event, and the aforementioned combinations cause it to add the word twice.
This appears to be a regression, since I don't observe this behavior in Firefox 3.6. And it impacts the Jetpack SDK's Widget API, which toggles the widget bar when the user presses Cmd+Shift+U. With two events dispatched, the bar toggles visible and then invisible again (or vice-versa), so the shortcut doesn't work.
![]() |
||
Comment 1•15 years ago
|
||
For whatever reason, I can reproduce the same with a Gecko 1.9.2 based Camino nightly – but no with the equivalent Namoroka build.
(and of course with Minefield recent nightlies)
Updated•15 years ago
|
blocking2.0: ? → betaN+
Comment 3•15 years ago
|
||
Who owns this? Need an owner ASAP.
Reporter | ||
Comment 4•15 years ago
|
||
Josh: is this something you could take on (or can you recommend someone else)?
Updated•15 years ago
|
Assignee: nobody → joshmoz
Turns out this is a pretty serious problem that gets to the heart of some fundamental flaws in our Cocoa key event propagation code. See bug 584965, which I just filed. That should resolve this.
The patch on bug 584965 fixes this and any other similar key combination problems.
I tried to write a chrome mochitest for this but the synthetic event apparently doesn't duplicate the incorrect behavior. Posting here in case anyone else can spot the problem.
Attachment #464673 -
Attachment is obsolete: true
Attachment #464885 -
Flags: review?
Attachment #464885 -
Flags: review? → review?(masayuki)
Comment 9•14 years ago
|
||
I tested the tests on my system. However, I cannot reproduce the failure when I send 0. Josh, how about on tryserver?
Assignee | ||
Comment 10•14 years ago
|
||
(In reply to comment #9)
> I tested the tests on my system. However, I cannot reproduce the failure when I
> send 0. Josh, how about on tryserver?
Without the patch on bug 584965 the following should happen in this test:
1. The ctrl-tab test should succeed.
2. The cmd+shift+a test should fail, it should get 2 events rather than the expected 1.
3. The cmd+; test should fail, it should get 2 events rather than the expected 1.
With the patch on bug 584965 all of this should succeed. The problem I'm having is that when I pass 0 instead of 1 for the keyboard layout I don't get the proper failures in an unpatched build, which means I don't know that the test is testing the right things.
Assignee | ||
Comment 11•14 years ago
|
||
Fixed on mozilla-central by the patch in bug 584965.
Still working on this test.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•14 years ago
|
||
Fix was backed out because of regressions.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 13•14 years ago
|
||
Comment on attachment 464885 [details] [diff] [review]
test v1.0
>+++ b/widget/tests/test_key_event_counts.xul
>@@ -0,0 +1,56 @@
>+ function onKeyPress(e)
>+ {
>+ gKeyPressEventCount++;
>+ }
I still recommend e.preventDefault(); here because somebody will add some tests which might cause later test failures.
>+ function runTest()
>+ {
>+ netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
>+
>+ var domWindowUtils = window.QueryInterface(Components.interfaces.nsIInterfaceRequestor).
>+ getInterface(Components.interfaces.nsIDOMWindowUtils);
>+
>+ window.addEventListener("keypress", onKeyPress, false);
>+
>+ var previousCount = gKeyPressEventCount;
>+
>+ // Test ctrl-tab
>+ domWindowUtils.sendNativeKeyEvent(1, 30, 0x0400, "\t", "\t");
>+ is(gKeyPressEventCount, ++previousCount);
Use 0 for keyboard layout and use 48 for keycode.
>+ // Test cmd+shift+a
>+ domWindowUtils.sendNativeKeyEvent(1, 0, 0x4000 | 0x0100, "a", "A");
>+ is(gKeyPressEventCount, ++previousCount);
Use 0 for keyboard layout but the keycode is correct.
>+ // Test cmd-;
>+ domWindowUtils.sendNativeKeyEvent(1, 29, 0x4000, ";", ";");
>+ is(gKeyPressEventCount, ++previousCount);
Use 0 for keyboard layout and use 41 for keycode.
The unexpected behavior is caused by the wrong keycodes.
And would you reset gKeyPressEventCound at starting each test? You increment the count by every test. That causes we'll see unclear test result, e.g., when a test failed but latter tests are passed, all of the tests are displayed as failed.
Assignee | ||
Comment 14•14 years ago
|
||
Comment on attachment 464885 [details] [diff] [review]
test v1.0
Thanks Masayuki. I got the key codes wrong because I was dumping them as hexadecimal instead of decimal!
Attachment #464885 -
Attachment is obsolete: true
Attachment #464885 -
Flags: review?(masayuki)
Assignee | ||
Comment 15•14 years ago
|
||
Fixed on mozilla-central by the patch in bug 584965.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•