Closed Bug 1426216 Opened 5 years ago Closed 5 years ago

Allow users to choose whether search suggestions or history suggestions come first in the address bar.

Categories

(Firefox :: Address Bar, enhancement, P1)

Product:
Firefox
Component:
Address Bar
Version:
57 Branch
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 59
Tracking Flags:
Tracking Status
relnote-firefox --- 59+
firefox58 --- unaffected
firefox59 --- verified

People

(Reporter: javaun, Assigned: adw)

References

(Depends on 1 open bug)

Details

(Whiteboard: [fxsearch])

Attachments

(2 files)

search_suggestions_pref.png
72.43 KB, image/png
Details
Bug 1426216 - Allow users to choose whether search suggestions or history suggestions come first in the address bar.
59 bytes, text/x-review-board-request
mak
: review+
Details 
In 57 we unified the address and search bars for new users. Some users may have a search-first workflow (we believe many Chrome users fall into this workflow) and some users have a history-first workflow (we believe most long-time Fx users fall into this workflow).

This work creates a UI setting under "Preferences > Search" to allow users to switch between search-first or history-first, just as we allow them to choose between unified vs. 2 bars. The target is Fx 59, and this has a new string dependency. 

Requirements:

1. Offer a new setting (see screencap) beneath the choice of unified vs. 2 bars. In a conversation with Past and Shorlander, we determined that the fastest/easiest implementation is a single checkbox so that we have a binary choice. 

* NOTE: while we will offer a binary choice to users (search-first or history-first), the underlying pref to change address bar composition (browser.urlbar.matchBuckets) is a string at could have many other values than the two we'll offer users via a UI pref. It's possible a power user changed this themselves, or it's possible that the user is part of a shield study where we're testing new combinations. 


2. The copy string is TBD and will say something like:

"[x] Prioritize search suggestions over history results in the address bar"

 MHeubusch will confirm the final string so that we can land in 59 ASAP.


In addition to this new user UI setting, there is some work to migrate/set this setting for new installs while keeping existing users exactly as they are:

3. Since 57, all new profile installs are given unification by default. These users should also get search-first results in the address bar. Currently we're giving these users search-first via Shield. This work should also properly set these new/unified users to the search-first underly pref which they can then change on their own via this new Preferences > Search setting.

4. All existing profiles were unchanged in 57, we did not unify their bar and they should also *keep* history results first. If those users want search-results first in the address bar, they can go into Preferences > Search and set that themselves.
Target Milestone: --- → Firefox 59
need copy string from Michelle
Flags: needinfo?(mheubusch)
We also need to confirm the actual configuration when search results are first, vs. when history is first. Stephen (NI) please confirm if these work

search-first (this is what we're currently setting for new profile installs in 57)

   browser.urlbar.matchBuckets  = suggestion:4, general:5 


history-first (this is status quo for legacy Fx users. They don't even have this pref set)

   browser.urlbar.matchBuckets = general:5 
  (or browser.urlbar.matchBuckets = general:5, suggestion:4)
Flags: needinfo?(shorlander)
We should keep the current config for now.
Flags: needinfo?(shorlander)
Recommended string: Show search suggestions ahead of browsing history in address bar results

Note there is no period at the end of this setting. Also please include this note for localization: This string describes what the user will observe when the system prioritizes search suggestions over browsing history in the results that extend down from the address bar. In the original English string, "ahead" refers to location (appearing most proximate to), not time (appearing before).
Flags: needinfo?(mheubusch)
Panos, I went ahead and made this string-only patch since we talked about this today and Michelle was able to recommend the string today too.
Assignee: nobody → adw
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [fxsearch]
Comment on attachment 8937884 [details]
Bug 1426216 - Allow users to choose whether search suggestions or history suggestions come first in the address bar.

https://reviewboard.mozilla.org/r/208572/#review214364
Attachment #8937884 - Flags: review?(past) → review+
Keywords: leave-open
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/806ebd3f042c
Allow users to choose whether search suggestions or history suggestions come first in the address bar. (String-only patch) r=past
(I didn't mean to post this r? as a new revision of the strings patch.)

The migration part of this is similar to the patch in bug 1423247.  Summary:

* Modifies UnifiedComplete.js to set the matchBuckets default to "suggestion:4,general:Infinity" under the assumption that new profiles will have a unified urlbar and will show search suggestions first by default.

* Adds a migration for this pref to nsBrowserGlue.  If the migration has already happened, does nothing.  Otherwise, if the pref value is set, then assume either the user set it or it was set by Shield to show search suggestions first.  Either way, do nothing.  Otherwise, look at the searchbar: if it's present, set the pref to show history first, and otherwise don't set the pref at all.

* Adds the checkbox to the search preferences UI using the string landed earlier.

* Adds a test for the UI.

* Adds a test for the migration.
Comment on attachment 8937884 [details]
Bug 1426216 - Allow users to choose whether search suggestions or history suggestions come first in the address bar.

https://reviewboard.mozilla.org/r/208572/#review217098

it looks mostly good, but I have a question

::: browser/components/nsBrowserGlue.js:2363
(Diff revision 2)
> +
> +    init() {
> +      if (Services.prefs.getBoolPref(this.migrated59PrefName, false)) {
> +        // Already migrated.
> +        return;
> +      }

There is just one thing I don't understand and it's why this is not a common migrateUI task, rather than running at every startup and bailing out on a custom pref.
Surely migrateUI may complicate things in case of an uplift, but that doesn't sound like the case and we could do this "every time check" only in the uplift. 

You could still keep the code in a separate helper and invoke it through the observer topic, so that should not be a problem.

Could you please clarify this choice?
You're right, I wasn't thinking.  New patch coming up soon.
Awesome work Drew! I have a question around migration. It sounds like we're not doing anything to the matchBuckets pref if the user is involved in the shield study. Is that correct? If so, I'm wondering if we can set the pref underneath shield. I'll explain...

Shield only sets pref values temporarily. At the end of a study, or if a user disenrolls (or is bumped), Shield cleans up and reverts to the current default value of any prefs it sets. Shield now has a new feature where, if the default value of a pref changes after the study is launched, it will "rollback" to the new default value. I'm not sure how this mechanism works, but our desired state (if feasible) if that for all new installs who got unified in 57 and who are seeing search-first in the Awesome Bar, that this will become their permanent state. 

Right now, the Shield study is setting matchBuckets pref for all new installs since 57 launch. If we aren't able to change the "default" version of this pref, then these users will rollback to history first when the experiment ends. (Cleanup reverts to default state, and at the time of 57 launch this matchBuckets pref didn't exist by default)

I could be misunderstanding. Please let me know! We can also NI Matt and Gregg Lind to understand the Shield rollback mechanism.



(In reply to Drew Willcoxon :adw from comment #11)
> (I didn't mean to post this r? as a new revision of the strings patch.)


> * Adds a migration for this pref to nsBrowserGlue.  If the migration has
> already happened, does nothing.  Otherwise, if the pref value is set, then
> assume either the user set it or it was set by Shield to show search
> suggestions first.  Either way, do nothing.  Otherwise, look at the
> searchbar: if it's present, set the pref to show history first, and
> otherwise don't set the pref at all.
>
Flags: needinfo?(adw)
(In reply to Javaun Moradi [:javaun] from comment #16)
> It sounds like we're not doing anything to the matchBuckets pref if the
> user is involved in the shield study. Is that correct?

Yes:  If the user is in the study, their pref will be set, and the migration code doesn't touch the pref if it's set.  They'll continue to see the same behavior.

I think we're good.  This patch leaves the default value of the pref as null/empty.  i.e., the default state for this pref still is that it does not exist.  What the patch does change is the interpretation of the null pref in the underlying urlbar code.  Currently, null is interpreted to mean, "show history first."  With this patch, it means, "show suggestions first."  That's the new default.

So say that we disenroll you from the study.  Your pref value will be reset to null/empty, it sounds like you're saying.  And say this bug lands in 59.  Let's look at a few scenarios.

(1) If you're disenrolled and stay with a pre-59 Firefox, you'll go back to the pre-59 default behavior of showing history first.

(2) If you're disenrolled, stay with a pre-59 Firefox, and later upgrade to 59, you will go through the migration step on upgrade.  When you start 59 the first time, if you're unified, then you'll get search suggestions first, and if you're not unified you'll get history first.

(3) If you are disenrolled after already having upgraded to 59, then you already went through the migration step, and it won't happen again.  You'll get search suggestions first regardless of your unified state.  (By default.  You can toggle the new checkbox in the UI if you want.)

Say that Shield does not roll back the pref when you're unenrolled after all.  Even then we should be good.  You'll continue to get search suggestions first because that's what the pref says should happen.  (Again, by default.)
Flags: needinfo?(adw)
Comment on attachment 8937884 [details]
Bug 1426216 - Allow users to choose whether search suggestions or history suggestions come first in the address bar.

https://reviewboard.mozilla.org/r/208572/#review217454

please fix UI_VERSION and we should be fine. Thank you.

::: browser/components/nsBrowserGlue.js:2274
(Diff revision 3)
> +      // urlbar results.
> +      this._migrateMatchBucketsPrefForUIVersion60();
> +    }
> +
>      // Update the migration version.
>      Services.prefs.setIntPref("browser.migration.version", UI_VERSION);

You forgot to bump UI_VERSION to 60, thus the added code currently runs at every startup.
Attachment #8937884 - Flags: review?(mak77) → review+
Should we have a relnote for this in 59?
Flags: needinfo?(jmoradi)
Mak, yes, I think we should do a relnote. I'm marking relnote-firefox-?. (Is that correct?) Also, NI Michelle. We can work with her to make sure it's explained clearly. 

ADW, thank you so much for the clarity. That approach makes sense, I misunderstood the mechanism. I think we're solid on migration.

Mak, I'm also glad you're doing reviews, since you advocated so correctly for not disturbing existing users workflows (i.e. keep 2-bar users history-first composition.) 

We'll get adequate testing on the edge cases to make sure that this is solid along these broad lines:
* The migration effectively makes concrete the "search-first" composition for those 57-onward new installs that got unified (via shield) and stayed with it.
* That we leave "history-first" for those legacy two-bar users who would not expect any changes to their Firefox.

Thanks all, I'm excited about this!
relnote-firefox: --- → ?
Flags: needinfo?(mheubusch)
Flags: needinfo?(mak77)
Flags: needinfo?(jmoradi)
Yep, the relnote just requires text to explain it, that Michelle can provide us. Thanks.
Flags: needinfo?(mak77)
Recommended note: For users with the unified search bar, search suggestions will appear ahead of browsing history by default.  This can be changed in Settings on Windows and in Preferences on Linux and MacOS. 


Javaun, please let me know if you'll be creating a post about this that I can link to from the release notes.
Flags: needinfo?(mheubusch)
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/440dbdb51f94
Allow users to choose whether search suggestions or history suggestions come first in the address bar. r=mak,past
(In reply to Drew Willcoxon :adw (Away 1/10–1/15) from comment #17)
> (In reply to Javaun Moradi [:javaun] from comment #16)
> > It sounds like we're not doing anything to the matchBuckets pref if the
> > user is involved in the shield study. Is that correct?
> 
> Yes:  If the user is in the study, their pref will be set, and the migration
> code doesn't touch the pref if it's set.  They'll continue to see the same
> behavior.
> 
> I think we're good.  This patch leaves the default value of the pref as
> null/empty.  i.e., the default state for this pref still is that it does not
> exist.  What the patch does change is the interpretation of the null pref in
> the underlying urlbar code.  Currently, null is interpreted to mean, "show
> history first."  With this patch, it means, "show suggestions first." 
> That's the new default.
> 
> So say that we disenroll you from the study.  Your pref value will be reset
> to null/empty, it sounds like you're saying.  And say this bug lands in 59. 
> Let's look at a few scenarios.
> 
> (1) If you're disenrolled and stay with a pre-59 Firefox, you'll go back to
> the pre-59 default behavior of showing history first.
> 
> (2) If you're disenrolled, stay with a pre-59 Firefox, and later upgrade to
> 59, you will go through the migration step on upgrade.  When you start 59
> the first time, if you're unified, then you'll get search suggestions first,
> and if you're not unified you'll get history first.
> 
> (3) If you are disenrolled after already having upgraded to 59, then you
> already went through the migration step, and it won't happen again.  You'll
> get search suggestions first regardless of your unified state.  (By default.
> You can toggle the new checkbox in the UI if you want.)
> 
> Say that Shield does not roll back the pref when you're unenrolled after
> all.  Even then we should be good.  You'll continue to get search
> suggestions first because that's what the pref says should happen.  (Again,
> by default.)

Javaun - It sounds like we should wait until the majority of users have updated to 59 and then kill the study. That would mean some users (non-updaters and slow updaters) would lose the search first experience at that point. Is that an acceptable risk?
Flags: needinfo?(jmoradi)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: leave-open
Resolution: --- → FIXED
Marking this fixed in 59 so it will show up more clearly on my release note triage.
status-firefox58: --- → unaffected
status-firefox59: --- → fixed
For now, this is in the release notes draft for 59.0b1 as New: "For users with the unified search bar, search suggestions will appear ahead of browsing history by default.  This can be changed in Settings on Windows and in Preferences on Linux and MacOS. "
relnote-firefox: ? → 59+
One more question: is this only affecting Firefox, or also Firefox for Android?
Flags: needinfo?(adw)
Depends on: 1429974
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #30)
> One more question: is this only affecting Firefox, or also Firefox for
> Android?

Only Firefox desktop
Flags: needinfo?(adw)
Hi Drew,

Does this require manual testing or it has been already covered?
Flags: needinfo?(adw)
Sure, there are a couple of STR to try.  "build1" means a build before this patch landed, and "build2" means a build after the patch landed:

STR 1

1. In build1, remove the search bar from the navbar if it's not already removed.
2. Quit build1, start build2.
3. Type "moz" in the urlbar, verify that search suggestions appear before bookmarks/history.
4. Add the search bar to the navbar.
5. Type "moz" in the urlbar, verify that search suggestions still appear before bookmarks/history.
6. Restart build2.
7. Type "moz" in the urlbar, verify that search suggestions still appear before bookmarks/history.
8. Go to about:preferences#search, uncheck "Show search suggestions ahead of..."
9. Type "moz" in the urlbar, verify that bookmarks/history appear before search suggestions.
10. Restart build2.
11. Go to about:preferences#search, verify that "Show search suggestions ahead of..." remains unchecked.
12. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.

STR 2

1. In build1, add the search bar to the navbar if it's not already added.
2. Quit build1, start build2.
3. Type "moz" in the urlbar, verify that bookmarks/history appear before search suggestions.
4. Remove the search bar from the navbar.
5. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
6. Restart build2.
7. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
8. Go to about:preferences#search, check "Show search suggestions ahead of..."
9. Type "moz" in the urlbar, verify that search suggestions appear before bookmarks/history.
10. Restart build2.
11. Go to about:preferences#search, verify that "Show search suggestions ahead of..." remains checked.
12. Type "moz" in the urlbar, verify that search suggestions still appear before bookmarks/history.
Flags: needinfo?(adw) → qe-verify+
Drew,

I started verifying this with Nightly build from 01.09.2018 as build1 and latest Nightly build as build2. For scenario 2, at step 3: "3. Type "moz" in the urlbar, verify that bookmarks/history appear before search suggestions." are you sure that bookmarks/history should appear before search suggestions? 

Please note that in about:preferences#search the "Show search suggestions ahead of browsing history in address bar results" is checked.
Flags: needinfo?(adw)
Yes, bookmarks/history should appear first because both the urlbar and search bar are in the navbar.  And the checkbox should be unchecked.  Are you sure you used a new profile when starting scenario 2?  If you did, then maybe a Shield study is interfering?  I'm not sure how that works with Nightly builds.
Flags: needinfo?(adw)
OK, it doesn't work right when I test it either.  I'll try to figure out what's going on.
Depends on: 1431489
Thanks, Brindusa.  I filed bug 1431489 for the follow-up fix.  This time I manually tested my own STR and verified the fix.  I posted try builds in the other bug.
(In reply to Drew Willcoxon :adw from comment #37)
> This time I manually tested my own STR and verified the fix.

But please verify it yourself, too!  Hence the try builds. :-)
Shouldn't be the preference "Show search suggestions ahead of browsing history in address bar results" go somewhere below "Provide search suggestions" and "Show search suggestions in address bar results"? To me it doesn't make much sense to decide about the search suggestions order/priority without knowing if they are even enabled.
At about:preferences#privacy, I have:

Address Bar
When using the address bar, suggest
☐ Browsing history
☑ Bookmarks
☑ Open tabs

So while the pref at about:preferences#search says:
Show search suggestions ahead of *browsing history* in address bar results

...for me it works as
Show search suggestions ahead of bookmarks and open tabs in address bar results

Is this expected?
(In reply to Bruce from comment #40)
> ...for me it works as
> Show search suggestions ahead of bookmarks and open tabs in address bar
> results
> 
> Is this expected?

The checkbox text is based on default preferences and a good explanation for the basic user ("browsing history" is easier to understand than "some local stuff").
It could be a little bit more generic but I'm not sure there's a good word for "results coming from a local source rather than a remote one". The only one that comes to my mind is "local matches", maybe too generic. All in all, the benefit seems very limited VS the cost of adapting the string for any case (unless we find a good way to express the locality of matches).

(In reply to Michal Stanke (Mozilla.cz) [:MikkCZ][:mstanke] (use needinfo) from comment #39)
> Shouldn't be the preference "Show search suggestions ahead of browsing
> history in address bar results" go somewhere below "Provide search
> suggestions" and "Show search suggestions in address bar results"? To me it
> doesn't make much sense to decide about the search suggestions
> order/priority without knowing if they are even enabled.

I think the pref is in the right place for our scopes, it's very related with the unified VS double bar behavior.
We could disable the checkbox if suggestions are disabled, but it may confuse the user, he may guess "why I can't change this pref?" and then it would need an explanation pointing at the other checkboxes below. In the end it's all in the same page so you have the situation outlined in front of you.
Again, the costs seem to overweight the benefit.
(In reply to Drew Willcoxon :adw from comment #38)
> (In reply to Drew Willcoxon :adw from comment #37)
> > This time I manually tested my own STR and verified the fix.
> 
> But please verify it yourself, too!  Hence the try builds. :-)

Hello Drew,

We verified this fix on Mac OS X 10.12, Windows 10 x64 and Ubuntu 16.04 using the STR 2 from comment 33 with the try build from bug 1431489 and we still reproduce this issue. Also as a note, when we start the build 2 the checkbox from "Show search suggestions ahead of browsing history in address bar results" is checked.
(In reply to ovidiu boca[:Ovidiu] from comment #42)
> We verified this fix on Mac OS X 10.12, Windows 10 x64 and Ubuntu 16.04
> using the STR 2 from comment 33 with the try build from bug 1431489 and we
> still reproduce this issue. Also as a note, when we start the build 2 the
> checkbox from "Show search suggestions ahead of browsing history in address
> bar results" is checked.

Hmm, I tried just now with the macOS try build and I could *not* reproduce the problem.  In other words, the STR 2 worked as expected.

This is the try build I used: https://queue.taskcluster.net/v1/task/K737-EG1QnuKy3V5D6s2EA/runs/0/artifacts/public/build/target.dmg

For build1, I used the 1/9 Nightly: http://ftp.mozilla.org/pub/firefox/nightly/2018/01/2018-01-09-10-01-17-mozilla-central/firefox-59.0a1.en-US.mac.dmg

Would you mind double-checking your results?
Flags: needinfo?(ovidiu.boca)
There's a lot of empty horizontal space up the top now. Can't search suggestions be displayed in a row, the way they are on mobile (and like the "search with" buttons on desktop)?
Hello Drew,

I double checked this on Mac OS X 10.12 and I can confirm the fix, the STR 2 works as expected.
Flags: needinfo?(ovidiu.boca)
I also did some exploratory testing and I think I have found an issue, Drew, please tell me your opinion. 

Scenario:

1. Open Firefox Nightly (in my case I used Nightly 59.0a1(2018-01-21))
2. Open the first tab and in URL bar write "moz"
3. Open the second tab and in URL bar write "cal" 
4. Open the third tab and in URL bar write "horse"
5. Open a new tab and in URL bar write one by one the 3 terms that where used above, "moz"; "cal"; "horse"

Actual results:

In the search suggestions, I see "cal"; "horse" but when I write "moz" nothing is displayed in the suggestions part. 

Expected results: 

When I write "moz" in the URL bar, this word should be displayed in the URL drop-down. 

NOTE: I did a screen recorder with this scenario: https://streamable.com/n8inh
Flags: needinfo?(adw)
Depends on: 1432263
I don't think there's any bug there.  With each of the three terms that you enter in the urlbar, you're shown bookmarks/history/tabs before search suggestions, which is the correct behavior since your checkbox is not checked.

There is a difference between the "moz" case and the others.  In that case, you're shown all the built-in Mozilla-related bookmarks because they match "moz", and since these bookmarks have a high frecency/priority, the "moz" Google search results page and tab are pushed out of the results and are not shown.

Please let me know if I'm misunderstanding.
Flags: needinfo?(adw)
Depends on: 1432716
I verified this issue across platforms (Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13.2) using the steps provided in comment 33, 59.0a1 (2017-12-21) as "build 1" and 59.0b5 (20180128191456) as "build 2". The expect result was triggered in every case. 
Considering these, the comment 47 and the fact that the behavior described in comment 46 is not reproducible on 59.0b5, I will mark this build as verified fixed.
Status: RESOLVED → VERIFIED
status-firefox59: fixed → verified
Flags: qe-verify+
Flags: needinfo?(jmoradi)
Flags: needinfo?(rrayborn)
Rob, Matt:
This may not go in 59. That would mean we need the search composition Shield study to continue.
Flags: needinfo?(mgrimes)
Depends on: 1444965
The studies have been modified to remain live for 59. Let us know once you have identified a new end date.
Flags: needinfo?(rrayborn)
Flags: needinfo?(mgrimes)
Reopening this meta bug as this feature is running the 60 trains. There are two major behaviors to be taken into account:

a. Profiles created on Fx 56/older: history/bookmarks are displayed ahead of suggestions (by default)
b. Profiles created on Fx 57/newer: search suggestions are displayed ahead of history/bookmarks (by default)
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Drew, what's the reason this stays open?
Flags: needinfo?(adw)
Cristian reopened it since it landed in 60 (comment 51).  I think QA wants to re-verify it on 60.  But you're right that it should be marked resolved at least.  Cristian, are you able to mark this as verified yet?
Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
Flags: needinfo?(adw) → needinfo?(cristian.comorasu)
Resolution: --- → FIXED
This feature shipped on Fx 60, we can close this issue.
Status: RESOLVED → VERIFIED
Flags: needinfo?(cristian.comorasu)
Hi,

I think this is not working as expected in 60.2.2esr (64-bit) running in Debian testing.

I have enabled "Provide search suggestions", "Show search suggestions on address bar" and disabled "search suggestions are displayed ahead of history/bookmarks", I added a bookmark with the tags "bugs kicad" and when I type "bugs kicad" in the address bar the search suggestion is shown first. Even after disabling "Provide search suggestions" the search suggestion is shown first.

See:
https://imgur.com/a/9i0dJB2


PD: Some checkbox labels may not be accurate, I'm using a non english firefox setup and the labels were translated by me.
Hello Fabian!

The behavior is intended. The first item in the drop down list displays the string you are searching and what search engine are you using, thus it is not a search suggestion.

Thank you!
QA Contact: mak77
QA Contact: mak77
Hi Cristian,

Is there a way to change that? I would prefer it to show the bookmarks first

Thanks.
There's no way to change it.  Typing a non-URL in the urlbar and hitting enter has always performed a search for what you typed (or at least since very early versions of Firefox), and the first, pre-selected result just shows you that that's going to happen.
Ok, thanks Drew!.
You need to log in before you can comment on or make changes to this bug.