1140440 - Mouse chooses options when search menu pops out under it

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[Wil Clouser [:clouserw]]

I'm on Nightly from Feb 11th with e10s enabled.  Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0

Rough steps to reproduce:

1) have mouse sitting near the top right of your browser window under where the search window will appear
2) ctrl-k to the search box and start typing
3) accidently bump your mouse over a pixel and notice it select something in the search box
4) hit enter to search and instead get whatever your mouse chose (including, potentially, a preferences panel)

See https://www.yammer.com/mozilla.com/#/Threads/show?threadId=506916390 for other discussion

Comment 1

[:Gijs (he/him)]

Philipp, thoughts?

Comment 2

[:Gijs (he/him)]

(FWIW, vaguely similar to bug 1043584)

Comment 3

[Mike Hommey [:glandium]]

You don't need 3) for 4) to happen. It happens to me too, on aurora 38 from 2-27. I never noticed it happening when aurora was on 37, but maybe I was lucky... (testing)... it happens on beta 37 too.

Comment 4

[Alice0775 White]

maybe Bug 1113731

Comment 5

[:Gijs (he/him)]

(In reply to Alice0775 White from comment #4)
> maybe Bug 1113731

This seems to only happen if you move the mouse, whereas the one-off and settings buttons can be invoked without moving the mouse, as comment #3 notes, which makes this a lot worse. :-\

Comment 6

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=34a66aaaca81&tochange=7c5f187b65bf

Comment 7

[:Gijs (he/him)]

	ab00a9cbd01b	Florian Quèze — Bug 1102038 - the 'Change Search Settings' button and the open search items cannot be used via the keyboard, r=Mossop.
	9b09899049a6	Florian Quèze — Bug 1126816 - a search started before the search panel opens goes to the previously selected one-off engine, r=Mossop.
	966573b47371	Florian Quèze — Bug 1124904 - cleanup keyboard navigation in the new search panel - separate the logic moving the selection from the logic examining at the keyboard events, r=Mossop.
	966ff0f3c21e	Florian Quèze — Bug 1124904 - cleanup keyboard navigation in the new search panel - simplify the 'selected' attribute handling, r=Mossop.
	365129a004d7	Florian Quèze — Bug 1124900 - add tests for keyboard navigation in the search panel, r=Mossop.

Delightful. Could be any of these. Probably doesn't reeeeaaaally matter too much.

Dave, do you have time to look at this as AIUI florian is not around at the moment?

Comment 8

[:Gijs (he/him)]

[Tracking Requested - why for this release]:
Serious regression regarding keyboard access to the search box.


A little bird told me Florian is also back now... I don't really care who takes this, but it would be nice if someone did look into it soon. :-)

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

I don't think this bug as described in comment 0 is that severe. However, comment 3 makes this worse. These are the steps that I used to reproduce with Gijs:

1. Type a search term in the search box.
2. Hover mouse of a search provider like Twitter
3. Without moving the mouse, clear the search box.
4. Enter a new term in the search box.

Actual:
Twitter (or the search provider that you selected) is still selected. 

Expected:
The search provider should be cleared.

Note that I cannot reproduce this bug on either Win7 or OSX by positioning the mouse where the Twitter search box is before the first search. In that case the search uses the default search provider. On Win7, the mouse remains in the foreground no matter what and looks to correctly select the one off search provider.

Tracking as the one off search provider should be cleared and this is a regression. flo-retina is going to investigate.

Comment 10

[Florian Quèze [:florian]]

Attachment: Patch

Comment 11

[Marco Mucci [:MarcoM]]

Hi Florian, can you provide a point value.

Comment 12

[:Gijs (he/him)]

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

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

This makes things better, although IMO we should still fix the underlying issue with mousemoves "overriding" keyboard selection as well.

Comment 13

[Florian Quèze [:florian]]

(In reply to :Gijs Kruitbosch from comment #12)

Thanks for the review!

> IMO we should still fix the underlying
> issue with mousemoves "overriding" keyboard selection as well.

I'm not sure yet there's general agreement for what the behavior should be there (although we all seem to agree that the current behavior is confusing), and I suspect the fix for this would be too scary for beta, so I would prefer we handle that case in a separate bug, and take a few more days to think it through.

Comment 14

[Florian Quèze [:florian]]

Try was green except for Mac failures that seem infra-related: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d41c99fd5ea

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

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c6fc46892109

Comment 16

[Florian Quèze [:florian]]

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

Approval Request Comment
[Feature/regressing bug #]: regression from bug 1124904
[User impact if declined]: possible to search with a non-default engine unintentionally if the mouse was under the area where the search panel will open.
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: low risk, self contained patch.
[String/UUID change made/needed]: none.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a3276215697d

Comment 18

[Lawrence Mandel [:lmandel] (use needinfo)]

This does look to be low risk but given that it is a change with our primary search UI, I would like to see the fix verified in m-c before uplift to Beta. We can consider this fix for Beta 6 on Monday.

Comment 19

[Petruta Horea [:phorea], Desktop QA]

This issue, as described in comment 0 is still reproducible on Nightly 39.0a1 and DevEd 38.0a2 2015-03-13 under Mac OSX 10.9.5, Ubuntu 12.04 32-bit and Win 7 64-bit.

On the other hand, it doesn't reproduce with the steps from comment 9. When the drop down is re-opened, the search engine is not highlighted anymore. But moving it hover another search engine and pressing Enter will show the bug.

Comment 20

[Florian Quèze [:florian]]

(In reply to Petruta Rasa [QA] [:petruta] from comment #19)
> This issue, as described in comment 0 is still reproducible on Nightly
> 39.0a1 and DevEd 38.0a2 2015-03-13 under Mac OSX 10.9.5, Ubuntu 12.04 32-bit
> and Win 7 64-bit.

We filed bug 1142334 to figure out what to do for that case.

> On the other hand, it doesn't reproduce with the steps from comment 9.

This is what we intended to fix here, marking verified :).

Comment 21

[Lawrence Mandel [:lmandel] (use needinfo)]

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

Now that the fix has been verified, let's get this into Beta 6. Beta+

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ec555c9715d2

Comment 23

[Petruta Horea [:phorea], Desktop QA]

Thanks, Lawrence!

Verified as fixed using Firefox 37 beta 6 on Mac OSX 10.9.5, Ubuntu 12.04 32-bit and Win 7 64-bit.

On Ubuntu and Mac, moving the mouse inside a search engine box doesn't highlight it, while on Windows, it's enough to move the mouse one pixel to see the issue.

Comment 24

[Mike Hommey [:glandium]]

It doesn't happen as reliably as before, but this is still happening.

Comment 25

[:Gijs (he/him)]

(In reply to Mike Hommey [:glandium] from comment #24)
> It doesn't happen as reliably as before, but this is still happening.

We fixed a specific subset of the problems. I think you're seeing bug 1113731 or bug 1139655. I'm going to re-close. Please reopen if there is really a specific different problem to be solved here.

Comment 26

[Mike Hommey [:glandium]]

I think I'm seeing something else than those two bugs you mention, but I'm not confident enough that it is different.

Comment 27

[Wil Clouser [:clouserw]]

The STR on the other two bugs are different (and seem more complex) than this one.  This one is simply:

1) Have mouse sitting in the area that the search drop down appears in
2) hit ctrl-k and start typing
3) hit enter and see your search query go to whatever your mouse was over instead of your defaults (including going to the settings page instead of searching)

From our POV, this bug is still happening, but if the internals say it's one of the other two bugs, I'll leave it to y'all.

Cheers

Comment 28

[Florian Quèze [:florian]]

I figured out in which case this is still happening. It's when while typing in the search field, the number of search suggestions available changes. That makes the bottom of the search panel move, and makes the mouse 'hover' some button.

I think the way forward is to just completely ignore selection done by mouse hover when the user presses enter. I'm going to try this in bug 1139655.