Closed Bug 1389221 Opened 4 years ago Closed 2 years ago

Some Emacs-like shortcuts not working in location bar on macOS

Categories

(Firefox :: Address Bar, defect, P3)

55 Branch
x86_64
macOS
defect

Tracking

()

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: mpolden, Assigned: ross.brandes)

References

Details

(Whiteboard: [fxsearch])

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170803105720

Steps to reproduce:

I type some letters in the location bar and a list of suggestions appears. I then press Ctrl+N or Ctrl+P to navigate to next or previous suggestion.


Actual results:

Ctrl+N does nothing. Ctrl+P unexpectedly navigates to the beginning of the input (like Ctrl+A).


Expected results:

On macOS, many Emacs-like keyboard shortcuts can be used in input fields. For example Ctrl+A and Ctrl+E for navigating to beginning and end of lie, respectively.

Ctrl+N should move focus to the next suggestion. Ctrl+P should move focus to previous suggestion. Both Safari and Chrome do this, and I expected Firefox to behave the same way.
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86_64
Component: Untriaged → Keyboard Navigation
Priority: -- → P3
Hi there, any chance this will get fixed for 57? I'm on the developer release switching from Chrome and, small as this is, might be the deal-breaker for me. Kind of jarring to be unable to use the common macOS / Emacs controls.
Flags: needinfo?(dao+bmo)
(In reply to Martin Polden from comment #0)
> Expected results:
> 
> On macOS, many Emacs-like keyboard shortcuts can be used in input fields.
> For example Ctrl+A and Ctrl+E for navigating to beginning and end of lie,
> respectively.

Does this only fail in the location bar or in all input fields in Firefox?
Flags: needinfo?(dao+bmo) → needinfo?(ksagar1030)
(In reply to Dão Gottwald [::dao] from comment #2)
> (In reply to Martin Polden from comment #0)
> > Expected results:
> > 
> > On macOS, many Emacs-like keyboard shortcuts can be used in input fields.
> > For example Ctrl+A and Ctrl+E for navigating to beginning and end of lie,
> > respectively.
> 
> Does this only fail in the location bar or in all input fields in Firefox?

It only fails in the location bar. The shortcuts work as expected in other input fields.
I suspect the problem is this controller:
https://dxr.mozilla.org/mozilla-central/rev/97efdde466f18cf580fda9673cf4c38ee21fc7b7/browser/base/content/urlbarBindings.xml#1038

It's only interested in customizing copy/cut commands but I guess it affects others too. What's the right way to implement this?
Component: Keyboard Navigation → Address Bar
Flags: needinfo?(ksagar1030) → needinfo?(ehsan)
Hmm, that controller looks good to me on a first glance at least, I can't think of anything obviously wrong with it.

Does the bug go away for example if you comment out <https://dxr.mozilla.org/mozilla-central/rev/97efdde466f18cf580fda9673cf4c38ee21fc7b7/browser/base/content/urlbarBindings.xml#88> and line 141?

A good way to debug this would be to set a breakpoint on nsXULControllers::GetControllerForCommand() and look for command names declared for bindings listed here: <https://searchfox.org/mozilla-central/rev/a4702203522745baff21e519035b6c946b7d710d/dom/xbl/builtin/emacs/platformHTMLBindings.xml#11>.
Flags: needinfo?(ehsan)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [fxsearch]
Does this bug need anything on my end? Unfamiliar with the process. I followed up on the initial reporting.
No, I think comment 5 should contain enough information for someone to start working on the bug.  Thanks for the report, Karan!
This is issue is important to me as well, so I dug into the code some and investigated. I apologize for the long length of this, but I hope it will help to provide a clearer path forward on this.

First, I tried commenting out https://dxr.mozilla.org/mozilla-central/rev/cc9e8157421130371eee2a782dd7b9363834e20d/browser/base/content/urlbarBindings.xml#88 and https://dxr.mozilla.org/mozilla-central/rev/cc9e8157421130371eee2a782dd7b9363834e20d/browser/base/content/urlbarBindings.xml#153 as comment 5 suggested. That had no effect on the ctrl-n/ctrl-p behavior.

I eventually discovered that the awesome bar is an HTML input: https://dxr.mozilla.org/mozilla-central/rev/cc9e8157421130371eee2a782dd7b9363834e20d/browser/base/content/urlbarBindings.xml#37. With that in mind, ctrl-n and ctrl-p are working in the default manner for an HTML input. In an HTML input (or a textarea with only one line), ctrl-p will take you to the beginning of the line, and ctrl-n will take you to the end of the line. This is the behavior for Safari and Chrome on macOS as well, not just Firefox. (If you're on macOS and would like to confirm this, I made a tiny CodePen example: https://codepen.io/anon/pen/vzGGNx.)

So as I understand it, it's not that there's any code that's preventing ctrl-n/ctrl-p from automatically working. Instead, there would need to be specific support added to make ctrl-n/ctrl-p work as expected (where "expected" is "matching the behavior of Safari and Chrome on macOS").

I'm not certain all of what would need to change, but I have a few ideas:
* The `onKeyPress` handler needs to note the `userSelectionBehavior` of using emacs-like keys. I believe this is used for analytics purposes. (reference: https://dxr.mozilla.org/mozilla-central/rev/cc9e8157421130371eee2a782dd7b9363834e20d/browser/base/content/urlbarBindings.xml#261)
* `_shouldDeferKeyEvent` needs to defer key events for ctrl-n. The tab and down arrow keys are deferred, so I'm sure ctrl-n would need to be too. This is complicated slightly by the fact that we'd need to defer on the "n" key only when control is pressed. That's not particularly hard in JS, but all the code here so far appears to have been written with a non-modified key press in mind. (references: https://dxr.mozilla.org/mozilla-central/rev/cc9e8157421130371eee2a782dd7b9363834e20d/browser/base/content/urlbarBindings.xml#313 and https://dxr.mozilla.org/mozilla-central/rev/cc9e8157421130371eee2a782dd7b9363834e20d/browser/base/content/urlbarBindings.xml#447)
* `nsAutoCompleteController::HandleKeyNavigation` (and other places?) needs to have a call signature that can handle taking in a modifier key (it currently only takes in the key code), and it needs to treat ctrl-p as `reverse` (I think?). (references: https://dxr.mozilla.org/mozilla-central/source/toolkit/components/autocomplete/nsAutoCompleteController.cpp#414 and https://dxr.mozilla.org/mozilla-central/source/toolkit/components/autocomplete/nsAutoCompleteController.cpp#450)

I have no experience in C++, so I'm not as certain of the changes that need to happen there, but I think the suggested changes I've listed should give a decent idea of the scope required, even if they're not 100% correct.

I'm open to working on this myself, but I have no experience in the Firefox codebase nor in C++, so I'd say it's unlikely that I'd be able to close out the feature entirely.
> 
> I eventually discovered that the awesome bar is an HTML input:
> https://dxr.mozilla.org/mozilla-central/rev/
> cc9e8157421130371eee2a782dd7b9363834e20d/browser/base/content/urlbarBindings.
> xml#37. With that in mind, ctrl-n and ctrl-p are working in the default
> manner for an HTML input. In an HTML input (or a textarea with only one
> line), ctrl-p will take you to the beginning of the line, and ctrl-n will
> take you to the end of the line. This is the behavior for Safari and Chrome
> on macOS as well, not just Firefox. (If you're on macOS and would like to
> confirm this, I made a tiny CodePen example:
> https://codepen.io/anon/pen/vzGGNx.)
> 

I am on macOS, and can confirm that Ctrl+p and Ctrl+n takes me to the beginning and to the end of a single line input, respectively.
I have completed a proof-of-concept commit that implements ctrl-n/ctrl-p for macOS (no C++ required!). Preferably, I'd like to get a review on my proof-of-concept. If the general direction is considered good, then I'd be happy to add all the necessary tests (hopefully with some guidance?).

I've never worked in the Firefox codebase before, so I'm unclear on what the next steps are. I don't know where to make my changes available for review, or who should be reviewing it. Can someone point me in the right direction?
Support ctrl-n and ctrl-p for navigating down and up (respectively) in the url
bar on macOS. The autocomplete widget will also support the same key bindings.
This functionality matches ctrl-n/ctrl-p behavior on the other major macOS
browsers.
Thanks for working on this!
Assignee: nobody → ross.brandes
Status: NEW → ASSIGNED
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/14e469a4365b
Add readline navigation bindings to url bar on macOS r=dao
https://hg.mozilla.org/mozilla-central/rev/14e469a4365b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Thank you so much for this! Just tested on Nightly and it works perfectly. World, rejoice! :D
Duplicate of this bug: 815735
Duplicate of this bug: 1389711
Thank you!
I'm not certain (and I don't have a Mac to test), but I suspect accessibility events won't be fired correctly when these keys are used. I fixed this behaviour for the up/down arrow keys and page up/down in bug 1331755, but I think there's going to need to be an additional tweak to handle these new keys. The handling for the arrow/page keys is here:
https://searchfox.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#322
(In reply to James Teh [:Jamie] from comment #19)
> I'm not certain (and I don't have a Mac to test), but I suspect
> accessibility events won't be fired correctly when these keys are used.


Thank you for bringing up that concern (and for the code reference!). I tested using VoiceOver on macOS, and the problem behavior in bug 1331755 does indeed crop up when using ctrl-n/ctrl-p. Should a new bug be opened? Or should a fix be performed against this bug? In either case, I'm happy to do the work to fix.
I'll open a new bug. Thanks for your fast response and willingness to fix.
Filed bug 1501904 re the accessibility issue.
Depends on: 1501904

This is also a bug under Linux (not just MacOS). Please if you could fix it, too.

You need to log in before you can comment on or make changes to this bug.