1444548 - Backout search composition work from 59

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Javaun Moradi [:javaun]]

In 57, we used Fx logic to show a single address bar (no search bar) for new installs. In 57 and beyond we've used Shield to put search suggestions first in the Awesome Bar for just those new installs (new profiles) who were unified in 57. The intention was to leave existing users.


In 59 we pursued work to add the search composition work to Firefox codebase. New profiles would get search-suggestions first. The intention was to leave existing users (pre 57 installs) as history-first.

We are seeing that existing profiles (prior to 57) who removed the search bar (use a single bar) are also affected and are getting search composition first. This could affect a large number of release users. 

The requirement is to back this work out of 59:

Back out: UI Pref to toggle history/search suggestions first
https://bugzilla.mozilla.org/show_bug.cgi?id=1432716

Back out: Logic to allow users to choose history/search first. Including migration path for users who 
https://bugzilla.mozilla.org/show_bug.cgi?id=1426216

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei or Florin, heads up that we will need some exploratory testing on your Monday morning, after this backout lands on m-r and we build RC3.  

I think mak or someone else from search engineering can describe here in the bug what behavior we want to see after the backout. 

If we see problems then this may delay the 59 release.

Comment 2

[Shilpi Gupta [:shilpi]]

Marco, Can you provide info on focus tests around the backed out regression? SV is expected to help QA this on Monday morning Romania time.

Comment 3

[Drew Willcoxon :adw]

Attachment: Release patch

(I'm getting timeouts trying to post this to mozreview, so I'm posting this directly to bugzilla instead.)

There are four bugs that need to be backed out:

1. https://hg.mozilla.org/releases/mozilla-release/rev/152720a4efe7 (bug 1432716)

2. https://hg.mozilla.org/releases/mozilla-release/rev/84a24eb373fa (bug 1432263)

3. https://hg.mozilla.org/releases/mozilla-release/rev/4d423fdc33dd (bug 1429974)

4. https://hg.mozilla.org/releases/mozilla-release/rev/440dbdb51f94 (bug 1426216)

The original bug implementing the problematic feature (pref migration + UI) is
4, and the others are improvements to it.

Comment 4

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Drew, just had a quick chat with Justin Dolske. I am around for another 2 hrs in case relman help is needed. In any case, given the critical nature of this backout, you have an A+ to land them to moz-release directly.

Comment 5

[Drew Willcoxon :adw]

https://hg.mozilla.org/releases/mozilla-release/rev/c61f5f5ead48c78a80c80db5c489bdc7cfaf8175

Comment 6

[Drew Willcoxon :adw]

Here are some STR to verify that the backout is good.  In summary, if you run 59 with a profile created pre-59, bookmarks/history should appear above search suggestions in the urlbar popup, regardless of whether the search bar is in your toolbar.  Same thing if you start 59 with a new profile.  (By 59 I mean the 59 build withg this backout patch, obviously.)

I verified these STR myself and I used Firefox 56 as the pre-59 version -- that is, the version where I created my "old" profile before using it in 59.  The reason I didn't use 58 is that a Shield study gets installed by default in 57 and 58 that makes the exact change that we're now trying to back out, and it's a pain to disable it. [1]

*** IMPORTANT: In the following STR, when you type "moz", the very first result in the popup will be "Search with Google".  That's OK and expected.  We're talking about all the results besides that one.

STR 1

1. In 56, remove the search bar from the toolbar if it's not already removed.
2. Quit 56, start 59.
3. Type "moz" in the urlbar, verify that bookmarks/history appear before search suggestions.
4. Add the search bar to the toolbar.
5. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
6. Restart 59.
7. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
8. Remove the search bar from the toolbar.
9. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.

STR 2

1. In 56, add the search bar to the toolbar if it's not already added.
2. Quit 56, start 59.
3. Type "moz" in the urlbar, verify that bookmarks/history appear before search suggestions.
4. Remove the search bar from the toolbar.
5. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
6. Restart 59.
7. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
8. Add the search bar to the toolbar.
9. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.

STR 3

1. Start 59 with a new profile.
2. Type "moz" in the urlbar, verify that bookmarks/history appear before search suggestions.
3. Add the search bar to the toolbar.
4. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
5. Restart 59.
6. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
7. Remove the search bar from the toolbar.
8. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.

STR 4

1. In 59, go to about:preferences#search (doesn't matter whether your profile is old or new).
2. Verify that there is no checkbox labeled "Show search suggestions ahead of browsing history in address bar results" above the "Default Search Engine" header
3. Verify that there's also no such checkbox under the "Provide search suggestions" checkbox

[1] Ideas from mythmon for disabling the Shield study in 57/58:

mythmon: two ideas: 1) you could make a new empty profile, add a prefs.js file with user_pref("extensions.shield-recipe-client.enabled", false) in it before you start
mythmon: that way shield will never run
mythmon: 2) if while shield is running you modify the pref under test (browser.urlbar.matchBuckets) to any user set value, shield will take this as a signal you don't want to be a part of this study, and permanently disable that study for your profile

Comment 7

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8957765 [details] [diff] [review]
Release patch

This is the main driver for RC3 build.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The shield study also gets installed in the Firefox 59 build.

For that reason,

> STR 3
> 
> 1. Start 59 with a new profile.
> 2. Type "moz" in the urlbar, verify that bookmarks/history appear before
> search suggestions.
> 3. Add the search bar to the toolbar.
> 4. Type "moz" in the urlbar, verify that bookmarks/history still appear
> before search suggestions.
> 5. Restart 59.
> 6. Type "moz" in the urlbar, verify that bookmarks/history still appear
> before search suggestions.
> 7. Remove the search bar from the toolbar.
> 8. Type "moz" in the urlbar, verify that bookmarks/history still appear
> before search suggestions.

fails if tested with a profile created in Firefox 56 without disabling Shield like this:

> [1] Ideas from mythmon for disabling the Shield study in 57/58:
> 
> mythmon: two ideas: 1) you could make a new empty profile, add a prefs.js
> file with user_pref("extensions.shield-recipe-client.enabled", false) in it
> before you start
> mythmon: that way shield will never run
> mythmon: 2) if while shield is running you modify the pref under test
> (browser.urlbar.matchBuckets) to any user set value, shield will take this
> as a signal you don't want to be a part of this study, and permanently
> disable that study for your profile

If you are quick after launching Firefox 59 with the new profile, you can see the expected behavior, but if you wait a few seconds after launching it, search suggestions are above bookmarks and history and about:config contains this:

browser.urlbar.matchBuckets;suggestion:4,general:5
extensions.shield-recipe-client.startupExperimentPrefs.browser.urlbar.matchBuckets;suggestion:4,general:5

The other tests passed with the build from the backout on mozilla-release: From Firefox 56 with shield enabled, from Firefox 58 with shield disabled.

Comment 9

[Drew Willcoxon :adw]

Sebastian, that sounds like the expected behavior for when the Shield study is installed.  This backout is independent of the Shield study: If the study is not installed, you should see the STR I posted; if the study is installed, then you'll see search suggestions first because that's what the study does.  Is there a problem here or am I misunderstanding you?

Thanks for pointing out that the study is installed on 59 too though.  That's something I didn't encounter because I was testing with my own build.  QA will need to take that into account when they test with the new RC build.

Comment 10

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #9)
> QA will need to take that into account when they test with the new RC build.

To be clear, here are the revised STR 3, taking into account that the Shield study gets installed in the RC:

STR 3

1. Create a new profile using 59 ***but do not run it or start Firefox normally with it***.
2. Add a prefs.js file to the new profile with user_pref("extensions.shield-recipe-client.enabled", false)
3. Start 59 with the new profile.
4. Type "moz" in the urlbar, verify that bookmarks/history appear before search suggestions.
5. Add the search bar to the toolbar.
6. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
7. Restart 59.
8. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.
9. Remove the search bar from the toolbar.
10. Type "moz" in the urlbar, verify that bookmarks/history still appear before search suggestions.

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Confirmed as working as expected for STR 3.

Comment 12

[Marco Bonardo [:mak]]

(In reply to Shilpi Gupta [:shilpi] from comment #2)
> Marco, Can you provide info on focus tests around the backed out regression?
> SV is expected to help QA this on Monday morning Romania time.

I'm available to support QA for the whole Monday, EU timezone, on IRC, mail or bugzilla.
Drew has posted good STR already.
Particular care must be taken with the Shield Study, and comment 10 has good suggestions on how to disable that.

Comment 13

[Cornel Ionce [:noni] [Hubs QA]]

Hi Marco!

We just created the Test Plan [1] and we could use your feedback before we start testing this.
Is there anything else you consider we should cover, besides what you've already suggested?

[1] https://tinyurl.com/ybs6yz3k

Comment 14

[Marco Bonardo [:mak]]

(In reply to Cornel Ionce [:cornel_ionce], Desktop Release QA from comment #13)
> We just created the Test Plan [1] and we could use your feedback before we
> start testing this.
> Is there anything else you consider we should cover, besides what you've
> already suggested?

As discussed over IRC, I'd repeat the testing that you plan on 56 also on 57 and 58.
An optional other test path could involve reopening the same profile with 56, then 57, then 58, then 59, to simulate a user who went through all of those versions (likely most).

Comment 15

[Marco Bonardo [:mak]]

I completed my personal testing session with build5 and everything looks good so far.
I stay available for questions from QA, the test plan looks good.

Comment 16

[Marco Bonardo [:mak]]

I think we can mark this as fixed now?