Closed Bug 1260820 Opened 4 years ago Closed 3 years ago

Search engine drop down closes when I change search engines by pressing Up/Down

Categories

(Firefox :: Preferences, defect)

46 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 50
Tracking Status
firefox46 --- wontfix
firefox47 --- wontfix
firefox48 + verified
firefox49 + verified
firefox50 --- verified

People

(Reporter: arni2033, Assigned: florian)

References

Details

(Keywords: regression)

Attachments

(1 file)

>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160326030430
STR:
1. Open about:preferences#search
2. Expand search engine drop down
3. Press Down key

AR:  Search engine drop down collapses
ER:  Search engine drop down should stay visible

This is regression from bug 1122618. Regression range:
> https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=861971c670e1fe27c8bf43833831d7ce17b46f00&tochange=503ebfe13cd2f079829f4a4c8b7e860b4989403f
Regression from 46, let's track this for 48/49 and find an owner for the bug.  
Wontfix for 46/47 as we are a couple of weeks away from the 47 release now. 

Florian, can you take a look here and help find an owner, since you were the reviewer on bug 1122618? Thanks!
Flags: needinfo?(florian)
Version: Trunk → 46 Branch
I can't reproduce on Mac. Someone else tested on a Linux nightly and couldn't reproduce either. This may be Windows specific.
I can reproduce on Windows.

On Mac when the user moves through the list in the drop down using up/down it highlights the new row but only selects it after pressing enter. On Windows it changes the selection immediately, which causes this code https://hg.mozilla.org/mozilla-central/rev/503ebfe13cd2#l1.33 to run and destroy the drop down to rebuild it.
Attached patch FixSplinter Review
I'm going to test this patch on Windows before requesting review, but I'm confident it should fix the problem.
Assignee: nobody → florian
Status: NEW → ASSIGNED
Comment on attachment 8761155 [details] [diff] [review]
Fix

I tested the patch on Windows and it worked as expected.
Flags: needinfo?(florian)
Attachment #8761155 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8761155 [details] [diff] [review]
Fix

A reluctant r=me as a wallpaper fix, but why does scrolling through that list fire engine-current on Windows but not elsewhere? Can't we just make it not do that? If not, please include a comment before the if() explaining why we need to check this...
Attachment #8761155 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #7)

> why does scrolling through that
> list fire engine-current on Windows but not elsewhere?Can't we just make it
> not do that? If not, please include a comment before the if() explaining why
> we need to check this...

I assumed it was an intended platform specific behavior. On Windows when going up/down in a menulist the highlighted row is selected immediately. On Mac when using up/down, there's a light highlight on the new row (similar to what we have for an hovered row).

I couldn't find code doing anything specific to Windows when looking at the menulist related code, but to be honest I didn't spend much time searching for it, because the patch doesn't seem bad to me: it's an optimization (we avoid rebuilding the whole list of engines) for the general case (ie. when the change has been made from the pref window, which should be the case most of the time).
https://hg.mozilla.org/integration/fx-team/rev/6e0788aa97c74aa11b75654bfc82d4ecb34f502a
Bug 1260820 - Search engine drop down closes when I change search engines by pressing Up/Down, r=Gijs.
https://hg.mozilla.org/mozilla-central/rev/6e0788aa97c7
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Comment on attachment 8761155 [details] [diff] [review]
Fix

Approval Request Comment
[Feature/regressing bug #]: regression from bug 1122618 in Firefox 46
[User impact if declined]: going through the dropdown of search engines in the preferences using the keyboard is painful on Windows.
[Describe test coverage new/current, TreeHerder]: landed on central, QA can verify.
[Risks and why]: low risk, the patch is simple, and self contained.
[String/UUID change made/needed]: none.
Attachment #8761155 - Flags: approval-mozilla-beta?
Attachment #8761155 - Flags: approval-mozilla-aurora?
Flags: qe-verify+
I've reproduced the initial issue on Nightly 2016-06-03  on Windows 7 64bit.

Verified fixed on Windows 7 64bit using latest Nightly 50.0a1 (buildID: 20160614030210).
Status: RESOLVED → VERIFIED
Comment on attachment 8761155 [details] [diff] [review]
Fix

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

This patch fixes the regression issue of search engine drop down issue and is verified. Take it in aurora & beta.
Attachment #8761155 - Flags: approval-mozilla-beta?
Attachment #8761155 - Flags: approval-mozilla-beta+
Attachment #8761155 - Flags: approval-mozilla-aurora?
Attachment #8761155 - Flags: approval-mozilla-aurora+
Verified fixed on Windows 7 64bit and Windows 10 64bit using latest Aurora 49.0a2 (buildID: 20160627004013) and Firefox 48 Beta 3 (buildID: 20160623122823).
See Also: → 1255513
Depends on: 1338104
You need to log in before you can comment on or make changes to this bug.