Closed Bug 1189101 Opened 4 years ago Closed 4 years ago

Cycling the default engine via the keyboard in the in-content search UI includes only one-click engines


(Firefox :: Search, defect, P3)




Firefox 48
Tracking Status
firefox42 --- affected
firefox48 --- fixed


(Reporter: nhnt11, Assigned: nhnt11)



(Whiteboard: [ui][fxsearch])


(1 file, 1 obsolete file)

Currently you can only cycle through engines that are visible as one-off buttons. As specified in
Flags: needinfo?(kev)
I've noticed that cycling through engines on about:newtab and about:home starts always with the first search engine from the list while on the search bar, it continues with the next search engine.

So if the 4th search engine is selected as default, Ctrl+Down Arrow will set the 5th one as default in search bar, while doing the same operation on those about: pages, the 1st search engine will be set as default.
Rank: 35
Priority: -- → P3
Attached patch WIP (obsolete) — Splinter Review
This fixes the bug, but I still need to check if there are any other consumers of ContentSearch.jsm's GetState API, and maybe update some comments. I'll request review after that.
Attachment #8712971 - Flags: feedback?(adw)
BTW, the patch also makes the behavior that Petruta described in comment 1 consistent with the main searchbar's.
Comment on attachment 8712971 [details] [diff] [review]

Review of attachment 8712971 [details] [diff] [review]:

Looks good to me.
Attachment #8712971 - Flags: feedback?(adw) → feedback+
I finally revisited this and don't think it needs any further changes. Carrying the f+ forward as r+.
Attachment #8712971 - Attachment is obsolete: true
Attachment #8728756 - Flags: review+
Comment on attachment 8728756 [details] [diff] [review]
Patch (updated a test, rebased)

My bad - this patch updates the browser_ContentSearch test (the WIP didn't do this).
Attachment #8728756 - Attachment description: Patch (same as WIP, rebased) → Patch (updated a test, rebased)
Attachment #8728756 - Flags: review+ → review?(adw)
Attachment #8728756 - Flags: review?(adw) → review+
Bug 1189101 - In-content search: include all engines when cycling with cmd+up/down. r=adw
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Flags: needinfo?(kev)
You need to log in before you can comment on or make changes to this bug.