1444042 - tabbrowser element removal seems to confuse EventUtils.synthesizeKey

Status: NEW

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

Description

[Dão Gottwald [:dao]]

This has caused two intermittent failures so far from what we know: bug 1443364 and bug 1443360.

My fix looks like this:

+  document.documentElement.focus();
   EventUtils.synthesizeKey("KEY_F11");

Here's my initial superficial guess how bug 1442651 could have caused this:

(In reply to Dão Gottwald [::dao] from comment #10)
> I'm going to attach a patch that fixes the test over here, but I really
> don't understand why. the only rational explanation I can come up with right
> now for how bug 1442651 broke this is that document.activeElement is the
> <browser> when sendWebSearchKeyCommand gets called first, whereas before bug
> 1442651 it probably was <tabbrowser> because the browser elements were
> anonymous. But the focused element should be the browser element regardless.
> I don't know why this would affect EventUtils.synthesizeKey, let alone why
> this would be intermittent. Like I said manually hitting accel+k worked
> fine, so I don't think we have a real regression here.

Enn, any further ideas about what's going on?

Comment 1

[Neil Deakin]

The keyboard shortcut handling shouldn't be affected by what has the focus. Do you know what actually is focused when it fails? Do different keydown/keypress events get fired in the fail case?

One possibility is that the checks for forwarding key events to the content process is failing to detect in some case that a remote browser is present (or not) and not propagating the key events properly.

Comment 2

[Dão Gottwald [:dao]]

The browser element was focused, so this sounds about right:

(In reply to Neil Deakin from comment #1)
> One possibility is that the checks for forwarding key events to the content
> process is failing to detect in some case that a remote browser is present
> (or not) and not propagating the key events properly.

Comment 3

[Dão Gottwald [:dao]]

Masayuki, Neil says you might understand this better...

Comment 4

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

Although I still don't understand.

It might detect some existing bugs around e10s. If <browser> has focus and it's a remote process's container, EventStateManager thinks its remote process has focus [1]. Then, it sends keyboard events to the remote process *after* dispatching the keyboard event in chrome DOM tree for checking if it's reserved by chrome [2]or needs reply from remote process [3]. Finally, the remote process will send reply event if it's requested by the chrome process [4].

If those shortcut keys are implemented only by JS with keydown or keypress event listener, the event won't be sent back to chrome process since JS cannot mark the event needs reply event. But I guess that they are implemented with <key> elements. If so, this is not the cause. Do you know if those shortcut keys are implemented with <key> elements?

1: https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/dom/events/EventStateManager.cpp#1369-1447
2: https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/dom/xbl/nsXBLWindowKeyHandler.cpp#544-556
3: https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/dom/xbl/nsXBLWindowKeyHandler.cpp#561-589
4: https://searchfox.org/mozilla-central/rev/a70da6775d5341a9cca86bf1572a5e3534909153/dom/ipc/TabChild.cpp#2092-2119

Comment 5

[Neil Deakin]

Yes, they are <key> elements.