Better Fallback for Command+K when search bar has been removed from the toolbar

RESOLVED FIXED in Firefox 52

Status

()

Firefox
Search
P3
normal
RESOLVED FIXED
10 months ago
4 months ago

People

(Reporter: jgruen, Assigned: Kaffeekaethe, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 52
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: [fxsearch])

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

10 months ago
Currently, I run Firefox with the search bar hidden in the "Additional Tools and Features" menu b/c I do not use it. According to SUMO the Command+K shortcut should focus the search bar, but with the search bar hidden it redirects the current tab to about:home. This is a highly destructive fallback behavior. 

STR
- Customize Firefox to hide the Search bar into the "Additional Tools and Features" section
- Navigate to any page
- hit Command+K
- That page redirects to about home and focuses the search field on that page

Expected
- About:home should be opened in a new tab so that this is a non-destructive operation
Imo, we should focus the locationbar in this case, not open about:home. But there may be reasons for doing that I'm not aware of.

Updated

10 months ago
Component: Keyboard Navigation → Search
Priority: -- → P3
Whiteboard: [fxsearch]

Updated

10 months ago
Mentor: dao+bmo@mozilla.com

Updated

10 months ago
Assignee: nobody → katharina.utecht
(Assignee)

Comment 2

10 months ago
Created attachment 8805759 [details] [diff] [review]
focusUrlBarIfSearchFieldIsNotActive.diff
Attachment #8805759 - Flags: review?(dao+bmo)
(Assignee)

Comment 3

10 months ago
Created attachment 8805764 [details] [diff] [review]
focusUrlBarIfSearchFieldIsNotActive and Tests.diff
Attachment #8805764 - Flags: review?(dao+bmo)
(Assignee)

Comment 4

10 months ago
Created attachment 8805779 [details] [diff] [review]
focusUrlBarIfSearchFieldIsNotActive and Tests.diff
Attachment #8805779 - Flags: review?(dao+bmo)
(Assignee)

Updated

10 months ago
Attachment #8805764 - Attachment is obsolete: true
Attachment #8805764 - Flags: review?(dao+bmo)
(Assignee)

Updated

10 months ago
Attachment #8805759 - Attachment is obsolete: true
Attachment #8805759 - Flags: review?(dao+bmo)

Comment 5

10 months ago
Comment on attachment 8805779 [details] [diff] [review]
focusUrlBarIfSearchFieldIsNotActive and Tests.diff

Looks good. We've discussed and reviewed this in person.

This also removes the behavior of focusing the search field in about:home or about:newtab when one of these pages is already loaded in the current tab. Instead we'll be consistent and always focus the location bar when we couldn't focus the search bar.
Attachment #8805779 - Flags: review?(dao+bmo) → review+
(Assignee)

Comment 6

10 months ago
Created attachment 8805781 [details] [diff] [review]
focusUrlBarIfSearchFieldIsNotActive and Tests.diff

Mit Meta Daten für CheckIn
Attachment #8805779 - Attachment is obsolete: true
Attachment #8805781 - Flags: review?(dao+bmo)

Comment 7

10 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/992fa87b5fbe
Make cmd k focus the urlbar if search bar is not present. r=Dao

Updated

10 months ago
Attachment #8805781 - Flags: review?(dao+bmo) → review+

Updated

10 months ago
Flags: in-testsuite+

Comment 8

10 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8109faaf35c0
Fix ESLint no-mixed-spaces-and-tabs failures.

Comment 9

10 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eab68afb0708
Fix indentation from whitespace fix.
Sorry, had to back this out for failing browser_901207_searchbar_in_panel.js, browser_urlbarFocusedCmdK.js and browser_urlbarHashChangeProxyState.js:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5fb5979e6bfb1e81637e0c5129e6e4f55567902c

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=992fa87b5fbee066579a359fcecd7d4b0054ff9f
Later push which run more tests and had more failing tests: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=e7c1f4f53f7c1e950f107e3dae0b4e5a93d237de

Failure logs:

https://treeherder.mozilla.org/logviewer.html#?job_id=38425027&repo=mozilla-inbound
[task 2016-10-29T14:47:18.555233Z] 14:47:18     INFO - TEST-START | browser/base/content/test/urlbar/browser_urlbarFocusedCmdK.js
[task 2016-10-29T14:48:03.581409Z] 14:48:03     INFO - TEST-INFO | started process screentopng
[task 2016-10-29T14:48:04.967464Z] 14:48:04     INFO - TEST-INFO | screentopng: exit 0
[task 2016-10-29T14:48:04.970988Z] 14:48:04     INFO - checking window state
[task 2016-10-29T14:48:04.972597Z] 14:48:04     INFO - Entering test bound 
[task 2016-10-29T14:48:04.974394Z] 14:48:04     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/urlbar/browser_urlbarFocusedCmdK.js | Test timed out - 

https://treeherder.mozilla.org/logviewer.html#?job_id=38425656&repo=mozilla-inbound
[task 2016-10-29T14:47:18.555233Z] 14:47:18     INFO - TEST-START | browser/base/content/test/urlbar/browser_urlbarFocusedCmdK.js
[task 2016-10-29T14:48:03.581409Z] 14:48:03     INFO - TEST-INFO | started process screentopng
[task 2016-10-29T14:48:04.967464Z] 14:48:04     INFO - TEST-INFO | screentopng: exit 0
[task 2016-10-29T14:48:04.970988Z] 14:48:04     INFO - checking window state
[task 2016-10-29T14:48:04.972597Z] 14:48:04     INFO - Entering test bound 
[task 2016-10-29T14:48:04.974394Z] 14:48:04     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/urlbar/browser_urlbarFocusedCmdK.js | Test timed out - 

Please fix the issues and request review for the updated patch. Thank you.
Flags: needinfo?(katharina.utecht)
(Assignee)

Comment 11

10 months ago
Created attachment 8805793 [details] [diff] [review]
patch_2.diff
Attachment #8805793 - Flags: review?(dao+bmo)

Updated

10 months ago
Depends on: 1314619

Comment 12

10 months ago
Comment on attachment 8805793 [details] [diff] [review]
patch_2.diff

To get this pass tests on Try, I had to tweak browser_urlbarFocusedCmdK.js a bit and disable browser_newtab_sponsored_icon_click.js (bug 1314619): https://treeherder.mozilla.org/#/jobs?repo=try&revision=2509f9be39712e5f5ff7f3ccb2dc10ad2c0dfbb9

I'll reattempt to land this with these changes.
Flags: needinfo?(katharina.utecht)
Attachment #8805793 - Flags: review?(dao+bmo) → review+

Comment 13

10 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5738d9db6102
Make accel+k focus the urlbar if search bar is not present. r=Dao

Comment 14

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5738d9db6102
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
(Reporter)

Comment 15

10 months ago

Updated

9 months ago
Depends on: 1315328

Comment 16

9 months ago
I was so happy with the old functionalitty :( It was the most used shortcut key for me :(

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

Comment 17

9 months ago
So now basically we have to keys that focus the URL bar that are exactly next to each other. Ctrl+K and Ctrl+L. What is the difference between them? Both K and L are on the home row (for Vim users) and instead of doing two different things they do the same thing. Why? How does this help anybody?

I understand that from time to time keys that I use very often will be removed. I get that. But having it do the same thing as it's neighbor is really silly. Please map Ctrl+K to something useful instead of focusing on the URL bar.

Comment 18

9 months ago
Also Ctrl+J does the same thing as Ctrl+K and Ctrl+L. :)
(In reply to andrei from comment #18)
> Also Ctrl+J does the same thing as Ctrl+K and Ctrl+L. :)

What OS are you using ?  Ctrl+J opens the Downloads Panel on Windows.

Comment 20

9 months ago
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #19)
> (In reply to andrei from comment #18)
> > Also Ctrl+J does the same thing as Ctrl+K and Ctrl+L. :)
> 
> What OS are you using ?

GNU/Linux.... Fedora 24.

> Ctrl+J opens the Downloads Panel on Windows.

And what does Control+Shift+Y do on Windows? :)

Do you have the search bar hidden? Maybe this is related.

Comment 21

9 months ago
Hmmm.... Brand new profile and both Ctrl+J and Ctrl+K do nothing. Ctrl + K should focus the URL bar after this fix, right?

Updated

9 months ago
Blocks: 1315509
Duplicate of this bug: 1183286
You need to log in before you can comment on or make changes to this bug.