1921161 - Add intercept keyboard event API for keyboard shortcut in browser (was Do not capture keyevent if webpage does not handle it)

Status: UNCONFIRMED

(GeckoView :: IME, enhancement, P3)

Description

[czlucius]

User Agent: Mozilla/5.0 (Android 14; Mobile; rv:132.0) Gecko/132.0 Firefox/132.0

Steps to reproduce:

This bug is related to https://bugzilla.mozilla.org/show_bug.cgi?id=1794664 in Fenix.

1. Press any key on an external keyboard.
2. The KeyEvent will be handled by onKeyDown, which after a few function calls, can be traced to https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java#2551 (processKey function).

It seems like the entry for GeckoView would be https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoView.java#880.

Actual results:

All KeyEvents that are valid are captured.
I.e:
The invalid keyevents/ones that should not be captured return false.
Afterwards, a Runnable is posted to (presumably to another thread) to handle the event. Then true is returned immediately.
Thus we do not know if the event is handled or not.

Expected results:

Only if the webpage overriden/used that specific shortcut, then we return true.
This blocks https://bugzilla.mozilla.org/show_bug.cgi?id=1794664 because the application does not know when the webpage has overriden or handled the shortcuts, hence cannot meaningfully implement keyboard shortcuts up to web standards.

Comment 1

[Thomas.Heinrich.Schmidt]

To whom it may concern: please review and set priority to P1-Fix in the current release cycle.
This is necessary so that after all these years Firefox on Android can finally be operated with an external keyboard.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:owlish, could you have a look please?

For more information, please visit BugBot documentation.

Comment 3

[Thomas.Heinrich.Schmidt]

Could you please update the severity?
This issue blocks the long-awaited “keyboard shortcut” improvement 1794664.

Comment 4

[Makoto Kato [:m_kato]]

I guess that parent process won't know whether consuming keyboard event on content process. So I guess that we must have same way of shortcut handling in Firefox desktop (XUL) then I might have an API (delegation?) for it.

Comment 5

[Makoto Kato [:m_kato]]

Nakano-san, I have a question. How handling of shortcut (in menubar) in Desktop? Before sending keyboard event to content process, does parent process handle shortcut of XUL menu?

Comment 6

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

First, shortcut keys are managed and handled by GlobalKeyListener which is attached to the root window or the XUL <keyset>.

In the parent process, it handles eKeyDown and eKeyPress. If the corresponding shortcut is "reserved", the event is marked and the event won't be sent to the focused remote process. Otherwise, the events are marked as "waiting for reply". Then, the key event will be sent to the focused remote process.

In the focused content process, the key event is dispatched into the DOM. Then, if the event is marked as "waiting for reply", it'll be sent back to the parent process.

Finally, in the parent process, the reply event is dispatched again in the DOM with marking it as "handled in remote process" and GlobalKeyListener executes the shortcut.

Comment 7

[czlucius]

Thanks Nakano-san for the detailed response.

It's definitely very helpful.
I'll go look into it (or maybe someone else too).

Thanks for taking your time to point me in the right direction!

Comment 8

[Thomas.Heinrich.Schmidt]

Does this mean that we do not only need to change the processKey function but need to have a complete new API?

Comment 9

[czlucius]

I think we have to expose some kind of callback when the events are not captured on the webpage, or reserved (e.g. Ctrl+T). I'll need to look into Gecko and GeckoView source code to see where I can implement this though.

Comment 10

[Makoto Kato [:m_kato]]

I don't think that no capture can implement browser's keyboard shortcut.

If browser implements shortcut, we should intercept keyboard handling before handling GeckoView / SessionTextInput.

So I think we should add some APIs (KeyboardIntercept?) in GeckoRuntime.

Comment 11

[Makoto Kato [:m_kato]]

(Or override onKJey* method in inherited GeckoView class in app)