1184220 - typing in the searchbar should de-select the settings or opensearch buttons at the bottom of the search panel

Status: VERIFIED FIXED

(Firefox :: Search, defect, P4)

Description

[:Gavin Sharp [email: [e-mail]]]

STR:
1) Type "some" in the search bar
2) Accidentally press the Up arrow
3) Type "thing" in the search bar
4) Press enter

Expected: Search for "something"
Actual: about:preferences#search opens

I've seen this happen with the down arrow too when there are no suggestions (which is a separate bug because that's not supposed to happen AFAIK), and a different search engine ends up being used than what was intended.

I can think of a few ways to address this:
a) make the non-autocomplete items in the dropdown "lose focus" when something is typed in the search bar
b) much broader: don't allow selection of non-autocomplete items with the up/down arrow

Comment 1

[Florian Quèze [:florian]]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #0)

> I've seen this happen with the down arrow too when there are no suggestions
> (which is a separate bug because that's not supposed to happen AFAIK)

Bug 1136139 made the 'no suggestions' case possible.

> I can think of a few ways to address this:
> a) make the non-autocomplete items in the dropdown "lose focus" when
> something is typed in the search bar
> b) much broader: don't allow selection of non-autocomplete items with the
> up/down arrow

I don't think the one-off engine buttons should get deselected when typing more. For the other items (eg. the open search items, and the settings item), it would make sense to unselect when more text is typed.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> Bug 1136139 made the 'no suggestions' case possible.

Even with suggestions disabled I usually get the thing that I am typing "suggested". But sometimes I run into the autocomplete popup being broken completely, for some reason. Haven't had a chance to debug.

> I don't think the one-off engine buttons should get deselected when typing
> more. For the other items (eg. the open search items, and the settings
> item), it would make sense to unselect when more text is typed.

It seems unlikely to me that there's a common use case that involves selecting a one-off button with the keyboard before you are done typing your search terms.

Comment 3

[Florian Quèze [:florian]]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #2)
> (In reply to Florian Quèze [:florian] [:flo] from comment #1)
> > Bug 1136139 made the 'no suggestions' case possible.
> 
> Even with suggestions disabled I usually get the thing that I am typing
> "suggested".

This was the expected behavior until Firefox 40. Starting with 41, we no longer intend to always show what you typed as a suggestion.

> But sometimes I run into the autocomplete popup being broken
> completely, for some reason. Haven't had a chance to debug.

Bug 1181868 ? This is a Nightly-only regression though.

> > I don't think the one-off engine buttons should get deselected when typing
> > more. For the other items (eg. the open search items, and the settings
> > item), it would make sense to unselect when more text is typed.
> 
> It seems unlikely to me that there's a common use case that involves
> selecting a one-off button with the keyboard before you are done typing your
> search terms.

It's something I'm unlikely to do, but I wouldn't be surprised if other people did it. I remember that back in December we were surprised by the feedback of people who for each search selected their search engine first and then started typing the query. This is currently possible, using <tab> you can select a one off engine, and then start typing.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to Florian Quèze [:florian] [:flo] from comment #3)
> Bug 1181868 ? This is a Nightly-only regression though.

Yes, could be.

> It's something I'm unlikely to do, but I wouldn't be surprised if other
> people did it.

"someone does it" shouldn't be the bar for keeping a feature. If it's much more likely to cause problems (as it has for me) than to be useful, then it's worth removing the "feature".

Comment 5

[Florian Quèze [:florian]]

Rephrasing the summary to match what I think the next step should be here.

Comment 8

[Florian Quèze [:florian]]

Attachment: Patch

Comment 9

[Florian Quèze [:florian]]

Comment on attachment 8658754 [details] [diff] [review]
Patch

Review of attachment 8658754 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/search/test/browser_searchbar_keyboard_navigation.js
@@ +162,5 @@
> +     "the settings item should be selected");
> +
> +  // Type a character.
> +  EventUtils.synthesizeKey("a", {});
> +  ok(!textbox.selectedButton, "search-settings",

The second parameter is unintended here, I've already removed it locally.

Comment 10

[:Gijs (he/him)]

Comment on attachment 8658754 [details] [diff] [review]
Patch

Review of attachment 8658754 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the local adjustment you noted in your comment.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/c3f13feea4cd

Comment 12

[Carsten Book [:Tomcat]]

sorry backed out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=4634684&repo=fx-team

Comment 13

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/8d4dfe0eb6c5

Comment 14

[Florian Quèze [:florian]]

(In reply to Carsten Book [:Tomcat] from comment #12)
> sorry backed out for failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=4634684&repo=fx-team

Turns out the patch we should have backed out was bug 1139655.

What happened is that the patch in bug 1139655 removed the workaround from bug 1140440 that was making the actual problem invisible. The actual problem is that browser_oneOffHeader.js was leaving the mouse pointer in a place where it would select one-off buttons as soon as the search panel would open, causing the keystrokes sent in browser_searchbar_keyboard_navigation.js to not select the expected items.

Here's a try push including a browser_oneOffHeader.js fix inside the patch from bug 1139655: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c05cec827bc7

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/27893f89ee39

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/27893f89ee39

Comment 17

[Nazir Ahmed Sabbir [:NaSb]]

Successfully reproduce this bug on Nightly 42.0a1 (2015-07-15) on Linux,64 bit with the comment 0's instruction!

This Bug is now verified as fixed on Latest Firefox Developer Edition 43.0a2 (2015-10-08)

Build ID 20151008004031
User Agent Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0

Comment 18

[Rezaul Huque Nayeem]

Reproduced the bug in 42.0a1 (2015-07-15) on windows 10 x64

Verified as fixed with latest Firefox Aurora 43.0a2 (Build ID: 20151008004031) 

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:43.0) Gecko/20100101 Firefox/43.0

As it is also verified on Linux (Comment 17), Marking it as verified!