Feature: Keyboard shortcuts
Categories
(Fenix :: General, task)
Tracking
(Not tracked)
People
(Reporter: csadilek, Unassigned, NeedInfo)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Attachments
(1 file)
98.34 KB,
image/jpeg
|
Details |
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/575Since most of those shortcuts would probably invoke UseCases (or indirectly perform things on the
SessionManager
orSession
).. 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
- [ ] 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 2•2 years ago
|
||
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•2 years ago
|
||
Some work was done on this, but needs help from a more experienced developer - see comments in:
Updated•2 years ago
|
Reporter | ||
Comment 5•2 years ago
|
||
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.
Reporter | ||
Comment 7•2 years ago
|
||
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.
Reporter | ||
Updated•2 years ago
|
I think View.OnKeyListener
and the corresponding setOnKeyListener
should be more appropriate for this use case.
(In reply to czlucius from comment #8)
I think
View.OnKeyListener
and the correspondingsetOnKeyListener
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.
Comment 10•2 years ago
|
||
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•2 years ago
|
||
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•11 months ago
|
||
How are things going? I would very much enjoy to be able to use Firefox on the tablet.
Comment 13•10 months ago
|
||
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•9 months ago
|
||
(In reply to czlucius from comment #11)
Created attachment 9324679 [details]
photo_2023-03-23_22-05-56.jpgI'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•5 months ago
|
||
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 hidden (advocacy) |
Comment 17•26 days ago
|
||
@czlucius Have you made any progress with your work?
Comment hidden (metoo) |
Comment 19•7 days ago
|
||
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•5 days ago
|
||
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•5 days ago
|
||
Comment 22•5 days ago
|
||
I am very, very satisfied. After all these years, you have dealt with the subject seriously, so that I can have some hope again.
Description
•