Closed Bug 1173745 Opened 4 years ago Closed 4 years ago

Add Search suggestion toggle to location bar preferences

Categories

(Firefox :: Address Bar, defect, P1)

defect
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: mak, Assigned: mak)

References

Details

(Whiteboard: [suggestions][fxsearch])

Attachments

(1 file, 1 obsolete file)

We need to deploy a first toggle for search suggestions in the awesomebar, the most coherent position for now is Options/Privacy/Location Bar, along with the other options.

We will likely make it more prominent in bug 959567.
Flags: qe-verify+
Flags: firefox-backlog+
Attached patch patch v1 (obsolete) — Splinter Review
I'm investigating an existing race condition in the handlers for these prefs, but it pre-existed to this bug, so I'm not going to block this on that, will file it apart and eventually fix it.
Attachment #8621166 - Flags: review?(adw)
Comment on attachment 8621166 [details] [diff] [review]
patch v1

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

Looks good, nice that we have tests for this and that they're easy to update.

::: browser/locales/en-US/chrome/browser/preferences/privacy.dtd
@@ +21,5 @@
>  <!ENTITY  locbar.bookmarks.label        "Bookmarks">
>  <!ENTITY  locbar.bookmarks.accesskey    "k">
>  <!ENTITY  locbar.openpage.label         "Open tabs">
>  <!ENTITY  locbar.openpage.accesskey     "O">
> +<!ENTITY  locbar.searches.label         "Search suggestions">

Only thing is that I think this wording is a little awkward.  Not by itself, but when viewed in the pane you see "When using the location bar, suggest... Search suggestions."  Suggest search suggestions.  I would suggest using "Searches from your default search engine."  Kind of long, but it's clear and unambiguous and uses the same terminology as the Search pref pane.

Anyhow, I would use the string I suggested but I'm not suggesting we should block the bug on it.  We can ask our copywriters, too.
Attachment #8621166 - Flags: review?(adw) → review+
Attached patch patch v1.1Splinter Review
as discussed on IRC, using "Relate_d_ searches from the default search engine"
Attachment #8621166 - Attachment is obsolete: true
Rank: 4
https://hg.mozilla.org/mozilla-central/rev/0462e431d7ac
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Depends on: 1180474
Marco, the same prefs used to enable "Related searches from the default search engine" about:preferences#privacy in Dev Edition 42.0a2, don't have the same effect in Firefox 41 beta 2.
Is there another pref available or it is enough if I check this bug on 42?
(In reply to Petruta Rasa [QA] [:petruta] from comment #8)
> Marco, the same prefs used to enable "Related searches from the default
> search engine" about:preferences#privacy in Dev Edition 42.0a2, don't have
> the same effect in Firefox 41 beta 2.
> Is there another pref available or it is enough if I check this bug on 42?

I'm not sure what you mean, firefox 41 is not a target milestone for the feature, it is still using the old autocomplete that doesn't care at all about the suggest.searches pref
So I can't verify this on Firefox 41? 
That's what I tried to do, since status-firefox for 41 is fixed. I checked on Firefox 42 and everything looks good there.
(In reply to Petruta Rasa [QA] [:petruta] from comment #10)
> So I can't verify this on Firefox 41? 

I'm not sure I see a reason to do that.
It should work but it's possible that 41 had some bugs that we fixed in 42 and thus you won't see what you expect there.
Thanks, Marco!

Removing qe-verify+ flag as per above comment. Also, bug 1181173 that already landed in 42.0a2 changed this behavior.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.