1308931 - Better Fallback for Accel+K when search bar has been removed from the toolbar

Status: RESOLVED FIXED

(Firefox :: Search, defect, P3)

Description

[John Gruen [:jgruen]]

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

Comment 1

[Marco Bonardo [:mak]]

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.

Comment 2

[Kaffeekaethe]

Attachment: focusUrlBarIfSearchFieldIsNotActive.diff

Comment 3

[Kaffeekaethe]

Attachment: focusUrlBarIfSearchFieldIsNotActive and Tests.diff

Comment 4

[Kaffeekaethe]

Attachment: focusUrlBarIfSearchFieldIsNotActive and Tests.diff

Comment 5

[Dão Gottwald [:dao]]

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.

Comment 6

[Kaffeekaethe]

Attachment: focusUrlBarIfSearchFieldIsNotActive and Tests.diff

Mit Meta Daten für CheckIn

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/992fa87b5fbe
Make cmd k focus the urlbar if search bar is not present. r=Dao

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8109faaf35c0
Fix ESLint no-mixed-spaces-and-tabs failures.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eab68afb0708
Fix indentation from whitespace fix.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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.

Comment 11

[Kaffeekaethe]

Attachment: patch_2.diff

Comment 12

[Dão Gottwald [:dao]]

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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5738d9db6102

Comment 16

[Andrei Petcu]

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

[Andrei Petcu]

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

[Andrei Petcu]

Also Ctrl+J does the same thing as Ctrl+K and Ctrl+L. :)

Comment 19

[Jim Jeffery not reading bug-mail 1/2/11]

(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

[Andrei Petcu]

(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

[Andrei Petcu]

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