1335347 - [e10s] `keyup` event for Alt key is not fired when menubar is activated by the Alt key press

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Description

[mz]

Attachment: aa.html

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.87 Safari/537.36

Steps to reproduce:

Press Alt
Press 1
Release 1
Release Alt

Attached is a simple onkeydown onkeyup logger.

I can reproduce consistently on 51.0.1 on Windows 32-bit.
The problem didn't occur (as far as there were no user reports) pre 51.
The problem doesn't occur on 51.0.1 on Linux 64-bit.






Actual results:

KeyDown Alt
KeyDown 1
KeyUp 1


Expected results:

KeyDown Alt
KeyDown 1
KeyUp 1
JeyUp Alt

Comment 1

[Loic]

I think it's buggy only with e10s enabled.

non-e10s result:
 keydown Alt { target: body, key: "Alt", charCode: 0, keyCode: 18 }  
 keydown Alt { target: body, key: "1", charCode: 0, keyCode: 49 }  
 keyup Alt { target: body, key: "1", charCode: 0, keyCode: 49 }  
 keyup { target: body, key: "Alt", charCode: 0, keyCode: 18 }

e10s result:
 keydown Alt { target: body, key: "Alt", charCode: 0, keyCode: 18 }  
 keydown Alt { target: body, key: "1", charCode: 0, keyCode: 49 }  
 keyup Alt { target: body, key: "1", charCode: 0, keyCode: 49 }

Comment 2

[Loic]

FF49 with e10s has the bug too.

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Stone, could you help take a look at this one? Thanks!

Comment 4

[Ming-Chou Shih [:stone]]

We prevent default on keyup event (key code=alt) in [1] and block dispatching it to content in [2].

[1] http://searchfox.org/mozilla-central/rev/12cf11303392edac9f1da0c02e3d9ad2ecc8f4d3/widget/windows/KeyboardLayout.cpp#1912
[2] http://searchfox.org/mozilla-central/rev/12cf11303392edac9f1da0c02e3d9ad2ecc8f4d3/dom/events/EventStateManager.cpp#1299

Comment 5

[Pierre Ossman]

Any progress? This (and bug 1435717) makes Firefox pretty much unusable with noVNC as it wedges the Alt key in the remote session.

Comment 6

[Pierre Ossman]

Any possible workarounds for us writing web apps until this is fixed?

Comment 7

[Pierre Ossman]

This is my plan at the moment:

https://github.com/novnc/noVNC/pull/1036

However it doesn't prevent the menu from popping up when Alt is pressed and released by itself. And it has a delay of course in detecting that Alt has been released.

Comment 9

[Matías Cánepa]

Is there any progress on this? Any target version for this to be solved?

I reported this on v58 and now I'm in v61 (https://bugzilla.mozilla.org/show_bug.cgi?id=1445724)

Comment 10

[Pierre Ossman]

I suspect this got fixed when bug 1435717 got fixed? So v63 would see the fix.

Comment 11

[Pierre Ossman]

I can confirm that this works in the nightly build, so I guess this bug should be closed with 63 as the target?

Comment 12

[Matías Cánepa]

That's good news!! Thank you Pierre

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1335347 - Make `nsMenuBarListener::KeyUp` wait replay event if a content process has focus r=smaug

keyup event of Alt key should be fired in content process even if it'll
activate the menubar, and it should be cancelable as same as before enabling
e10s.

Unfortunately, the new test couldn't be written simpler than what it is in
this patch. When I write it with ContentTask.spawn with
ContentTaskUtils.waitForEvent, it sometimes couldn't receive keyup event
in the content process. Therefore, for avoiding intermittent timeout caused
by it, the test in this patch retries 5 times if Alt key events are not
fired in the content process.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

smaug: ping for review. I guess it's dropped from your queue accidentally.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

It is still there in the queue :)
Review queue was just rather long after my vacation. Sorry about the delay.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e176bd45edd8
Make `nsMenuBarListener::KeyUp` wait replay event if a content process has focus r=smaug

Comment 19

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/e176bd45edd8

Comment 20

[Andronachi Marian]

Nu sure if exists but just a idea is if possible an option in about:config to disable the menu who not what it ?

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Andronachi Marian from comment #20)

Nu sure if exists but just a idea is if possible an option in about:config to disable the menu who not what it ?

"ui.key.menuAccessKeyFocuses"

Comment 22

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures in browser_alt_keyup_in_content.js

backout: https://hg.mozilla.org/mozilla-central/rev/6bf980923afe9a1c1f26929b4b188558be85b114

push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=e176bd45edd8b076190a341010aca3224cd47cd0&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

failure log: https://treeherder.mozilla.org/logviewer?job_id=322249074&repo=autoland&lineNumber=5156

[task 2020-11-19T01:12:22.258Z] 01:12:22 INFO - TEST-PASS | dom/events/test/browser_alt_keyup_in_content.js | Testing menubar is shown by Alt keyup: Menubar should've been activated by the synthesized Alt key press -
[task 2020-11-19T01:12:22.258Z] 01:12:22 INFO - Buffered messages logged at 01:12:17
[task 2020-11-19T01:12:22.258Z] 01:12:22 INFO - Testing menubar is shown by Alt keyup: Inactivating the menubar...
[task 2020-11-19T01:12:22.258Z] 01:12:22 INFO - Buffered messages finished
[task 2020-11-19T01:12:22.259Z] 01:12:22 INFO - TEST-UNEXPECTED-FAIL | dom/events/test/browser_alt_keyup_in_content.js | Aborting this test due to unexpected the exception (Testing menubar is shown by Alt keyup: Waiting for closing all popups - timed out after 50 tries.) -
[task 2020-11-19T01:12:22.259Z] 01:12:22 INFO - Stack trace:
[task 2020-11-19T01:12:22.259Z] 01:12:22 INFO - chrome://mochikit/content/browser-test.js:test_ok:1304
[task 2020-11-19T01:12:22.259Z] 01:12:22 INFO - chrome://mochitests/content/browser/dom/events/test/browser_alt_keyup_in_content.js:runTests:271
[task 2020-11-19T01:12:22.259Z] 01:12:22 INFO - Leaving test bound runTests
[task 2020-11-19T01:12:22.259Z] 01:12:22 INFO - GECKO(9692) | MEMORY STAT | vsize 2104241MB | vsizeMaxContiguous 67431177MB | residentFast 243MB | heapAllocated 76MB
[task 2020-11-19T01:12:22.259Z] 01:12:22 INFO - TEST-OK | dom/events/test/browser_alt_keyup_in_content.js | took 5779ms

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b8265dc60c8
Make `nsMenuBarListener::KeyUp` wait replay event if a content process has focus r=smaug

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Sigh. browser-chrome framework is really buggy according to the log.

[task 2020-11-19T01:12:16.471Z] 01:12:16     INFO - TEST-START | dom/events/test/browser_alt_keyup_in_content.js
[task 2020-11-19T01:12:22.193Z] 01:12:22     INFO - TEST-INFO | started process screenshot
[task 2020-11-19T01:12:22.255Z] 01:12:22     INFO - TEST-INFO | screenshot: exit 0
[task 2020-11-19T01:12:22.255Z] 01:12:22     INFO - Buffered messages logged at 01:12:16
[task 2020-11-19T01:12:22.255Z] 01:12:22     INFO - Entering test bound runTests
[task 2020-11-19T01:12:22.256Z] 01:12:22     INFO - TEST-PASS | dom/events/test/browser_alt_keyup_in_content.js | This test assumes that Alt key activates the menubar - 
[task 2020-11-19T01:12:22.256Z] 01:12:22     INFO - Start to test: Testing menubar is shown by Alt keyup...
[task 2020-11-19T01:12:22.256Z] 01:12:22     INFO - Testing menubar is shown by Alt keyup: Waiting browser getting focus...
[task 2020-11-19T01:12:22.257Z] 01:12:22     INFO - Testing menubar is shown by Alt keyup: Waiting content process getting focus...
[task 2020-11-19T01:12:22.257Z] 01:12:22     INFO - Testing menubar is shown by Alt keyup: Waiting keyup events of Alt in chrome...
[task 2020-11-19T01:12:22.258Z] 01:12:22     INFO - Console message: [JavaScript Warning: "Key event not available on some keyboard layouts: key=“i” modifiers=“accel,alt,shift” id=“key_browserToolbox”" {file: "chrome://mochikit/content/tests/SimpleTest/EventUtils.js" line: 1210}]
[task 2020-11-19T01:12:22.258Z] 01:12:22     INFO - Testing menubar is shown by Alt keyup: Waiting keyup event in content...
[task 2020-11-19T01:12:22.258Z] 01:12:22     INFO - A popup is hidden (visible popups: -1)
[task 2020-11-19T01:12:22.258Z] 01:12:22     INFO - TEST-PASS | dom/events/test/browser_alt_keyup_in_content.js | Testing menubar is shown by Alt keyup: Menubar should've been activated by the synthesized Alt key press - 
[task 2020-11-19T01:12:22.258Z] 01:12:22     INFO - Buffered messages logged at 01:12:17
[task 2020-11-19T01:12:22.258Z] 01:12:22     INFO - Testing menubar is shown by Alt keyup: Inactivating the menubar...
[task 2020-11-19T01:12:22.258Z] 01:12:22     INFO - Buffered messages finished
[task 2020-11-19T01:12:22.259Z] 01:12:22     INFO - TEST-UNEXPECTED-FAIL | dom/events/test/browser_alt_keyup_in_content.js | Aborting this test due to unexpected the exception (Testing menubar is shown by Alt keyup: Waiting for closing all popups - timed out after 50 tries.) - 
[task 2020-11-19T01:12:22.259Z] 01:12:22     INFO - Stack trace:
[task 2020-11-19T01:12:22.259Z] 01:12:22     INFO - chrome://mochikit/content/browser-test.js:test_ok:1304
[task 2020-11-19T01:12:22.259Z] 01:12:22     INFO - chrome://mochitests/content/browser/dom/events/test/browser_alt_keyup_in_content.js:runTests:271

[task 2020-11-19T01:12:22.258Z] 01:12:22 INFO - A popup is hidden (visible popups: -1)

This means that a popup was open at starting the test, i.e., previously running test nor the framework didn't clean up the previous test's popup. That may cause intermittent failure of the new test anyway. I'll modify it to ignore unexpected popup hidden events for avoiding the timeout, but another intermittent failure could happen.

Comment 25

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5b8265dc60c8