1794664 - Feature: Keyboard shortcuts

Status: NEW

(Fenix :: General, task)

Description

[Christian Sadilek [:csadilek]]

From github: https://github.com/mozilla-mobile/android-components/issues/1978.

This came up for the reference browser here:
https://github.com/mozilla-mobile/reference-browser/issues/575

Since most of those shortcuts would probably invoke UseCases (or indirectly perform things on the SessionManager or Session).. maybe we could just wrap that in a component and with that provide those keyboard shortcuts to all our apps easily.

Related code in Fennec:
https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#471-567

https://searchfox.org/mozilla-esr68/rev/13df4aebd54ee9ba73924dd558af289ef515a490/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#534

---

• [ ] New tab
• [ ] Navigate forward
• [ ] Navigate back
• [ ] Reload
• [ ] Stop loading
• [ ] Private tab
• [ ] Close tab
• [ ] Find in page

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[edwin]

Would love to have this feature back.

Comment 2

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

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

For more information, please visit auto_nag documentation.

Comment 3

[David Balch]

Some work was done on this, but needs help from a more experienced developer - see comments in:

• https://github.com/mozilla-mobile/android-components/issues/1978
• https://github.com/mozilla-mobile/android-components/pull/9948

Comment 5

[Christian Sadilek [:csadilek]]

See also: https://github.com/mozilla-mobile/fenix/issues/3729

Comment 6

[czlucius]

Maybe we can have something like this (UML diagram attached)?
https://user-images.githubusercontent.com/58442255/224989808-3c256caf-8738-4e9d-808c-f728ff4981d0.png

From @gaul's code (attached GitHub issue), we can have a generic abstract DispatchKeyEvent (is that alr in Fenix?), that would be extended to implement a single shortcut for every subclass (for extensibility, if not all shortcut code must be added to only one file) - similar to what is done for Feature(I think?)
We can then have an ArrayList<DispatchKeyEvent> in the GeckoEngineView, then in the BaseBrowserFragment, or appropriate locations, we add the shortcuts one by one with addShortcut().

Also, if needed, I can also add a field for the name and description of the shortcut, so users can discover is more quickly. We should use the OS keyboard shortcuts menu to advertise available shortcuts, probably in HomeActivity. Although undocumented, it is a feature of AOSP since Android Nougat, and I've discovered how to do this and posted my answer here: https://stackoverflow.com/a/71062698/12204281.

How does this sound?
Thanks.

Comment 7

[Christian Sadilek [:csadilek]]

Reading the original discussion on Github, I agree that's roughly the right shape.

The missing piece here is the Feature class, which would allow registering specific shortcut handlers in applications e.g., Fenix. The feature class would be bound to the view (as most of our features), have access to an EngineView and listen for key events. It would then invoke the corresponding shortcut handler in response to those events.

Comment 8

[czlucius]

I think View.OnKeyListener and the corresponding setOnKeyListener should be more appropriate for this use case.

Comment 9

[czlucius]

(In reply to czlucius from comment #8)

I think View.OnKeyListener and the corresponding setOnKeyListener should be more appropriate for this use case.
Seems to be what Fennec uses as well, although Fennec hard-codes it. I think a feature implementation will do well for this.

https://hg.mozilla.org/mozilla-central/file/add212ee1118aff550a5fc05420282556fb3ade9/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#l573

Comment 10

[czlucius]

Seems like onKeyDown, onKeyUp and onKeyPreIme is overriden in GeckoView.java, hence nothing is received with setOnKeyListener.
Thus, an override for dispatchKeyEvent is needed.

Comment 11

[czlucius]

Attachment: photo_2023-03-23_22-05-56.jpg

I've started working on this, got the reload shortcut to work with the suggested modular approach, implemented with a feature component.
Will push a PR when it is ready.

Comment 12

[gitta.hubner]

How are things going? I would very much enjoy to be able to use Firefox on the tablet.

Comment 13

[gustavo]

Hello there,

Just want to add some extra context as reading here makes it look like a relativeky new problem, but this user request dates back 12 years now, if we count it all the way to the Fennec issue that was fixed shortly before the transition to Fenix, which still has the bug to this day:

https://bugzilla.mozilla.org/show_bug.cgi?id=726716

This was also discussed to some length in GitHub for over 4 years, before the repository was retired:

https://github.com/mozilla-mobile/fenix/issues/3729

That latter issue is linked from user comments all around the web, mainly as people find that they are stuck with other browsers whenever they want to use Android with a keyboard, which is more common nowadays as there are larger tablet screens.

It's great to see some action on this per earlier comments above, but considering it's been over 8 months now, hopefully this didn't get unintendedly missed again as other items moved up the list.

Thanks for the great work you all do.

Comment 14

[christoph]

(In reply to czlucius from comment #11)

Created attachment 9324679 [details]
photo_2023-03-23_22-05-56.jpg

I've started working on this, got the reload shortcut to work with the suggested modular approach, implemented with a feature component.
Will push a PR when it is ready.

Hi czlucius, your screenshot looks great! Did you have a chance to move this topic forward in the past few months?

The missing support for keyboard shortcuts on Android is a real annoyance. It often leads me to to go back to Chrome on my Samsung Tab S9 tablet with the keyboard cover.

Comment 15

[Thomas.Heinrich.Schmidt]

Is there any hope that keyboard shortcuts will be implemented for Android Firefox?
Maybe someone with the required superpower can review Priority: Not set Severity: N/A.
Otherwise, this issue probably has the last place in the ranking.

Comment 16

[gitta.hubner]

This is such an unbelievably low hanging fruit. Yet it was not fixed for over years, preventing a percent or so of users from using firefox. Is the bugtracking system dysfunctional in prioritizing? Everybody please just make this happen, karma nirvana is all yours.

Comment 17

[Thomas.Heinrich.Schmidt]

@czlucius Have you made any progress with your work?

Comment 18

[Veddu]

I just came here to see what the status of this request was. Had to go back to vivaldi since I neither have tabs nor keyboard shortcuts when using samsung dex..

Comment 19

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

Redirect a needinfo that is pending on an inactive user to the triage owner.
:boek, since the bug has recent activity, could you have a look please?

For more information, please visit BugBot documentation.

Comment 20

[czlucius]

Hi there.
I have been quite busy the past year, but have found some time on this of late. .

From what I can see from the attempted PR and Fennec, they all handle dispatchKeyEvent, and I think the ideal place is in GeckoEngineView.
I have made slight progress in the keyboard feature in AC, to plug into a GeckoEngineView for capturing dispatched KeyEvents.
Some of the keyboard shortcuts can be written in AC in the Feature itself, but some (like new tab) have to be written in Fenix as it is very dependent on navigation.

But before this can be solved, there is 1 problem that will be blocking it (in which I can file another issue), if not even if keyboard shortcuts are implemented the experience would not be according to standard:
Some websites may call event.preventDefault() to capture the shortcut from the browser, and we should support this behaviour (see https://stackoverflow.com/a/3680946/12204281). It seems like for GeckoView's processing keyevent code which leads to GeckoEditable.java (in the fn processKey, around line 2551), just returns true (hence capturing the keyevent), for all valid keyboard shortcuts (see snippet below).
This means all keyevents including our keyboard shortcuts is captured by Gecko, and its either all (we capture all shortcuts and disallow any website-override), or nothing (the browser does not get any keyevent at all because it is captured by Gecko).

Hence I think we need to find a solution to get Gecko(View) to return us whether it has captured the keyevent (i.e. the website has overriden it), which will be the return value of processKey hence onKeyDown (in GeckoView.java, line 880). I don't have much experience with Gecko code, it seems like afterwards it is being passed to native code in C++ which I cannot trace. But the chain of functions all return void which is going to be a problem (in addition to the callback-based sendKeyEvent as seen below)? Maybe someone from the Fenix/GeckoView team can advice on how we can proceed with this.

Thanks!
Let me know if you need any clarification.

Snippet for processKey of GeckoEditable:

  private boolean processKey(
      final @Nullable View view,
      final int action,
      final int keyCode,
      final @NonNull KeyEvent event) {
    if (keyCode > KeyEvent.getMaxKeyCode() || !shouldProcessKey(keyCode, event)) {
      return false;
    }

    postToInputConnection(
        new Runnable() {
          @Override
          public void run() {
            sendKeyEvent(view, action, event);
          }
        });
    return true;
  }

Comment 21

[czlucius]

The function can be found here:
https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java#2551

Comment 22

[Thomas.Heinrich.Schmidt]

I am very, very satisfied. After all these years, you have dealt with the subject seriously, so that I can have some hope again.