1280381 - The prompt to enable search suggestions in the awesomebar panel should not be shown more than a few times

Status: VERIFIED FIXED

(Firefox :: Search, defect, P1)

Description

[Florian Quèze [:florian]]

We have Telemetry data showing that a very large proportion of our users never answer the Yes/No question in this prompt, and that it just stays permanently in their way.

After it has been shown several times, we should automatically hide it and assume the answer is "No".
It's possible for the awesomebar panel to be shown and hidden several times quickly without the user necessarily having time to read immediately (if they type quickly in the location bar and press enter), so I think we should ensure it's shown eg. on 3 different days before we hide in automatically.

Comment 1

[Marco Bonardo [:mak]]

[Tracking Requested - why for this release]: The prompt is part of the awesomebar redesign we are shipping in 48. The current situation us sub-optimal, since we keep showing the prompt forever, and we noticed how many users are never making a decision. We would like to hide the prompt after a while for those users.

Comment 2

[Florian Quèze [:florian]]

Attachment: The prompt to enable search suggestions in the awesomebar panel should not be shown more than a few times,

I wonder if this needs a test or if this can be covered by QA only.

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8764329 [details] [diff] [review]
The prompt to enable search suggestions in the awesomebar panel should not be shown more than a few times,

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

::: browser/app/profile/firefox.js
@@ +293,5 @@
>  pref("browser.urlbar.suggest.searches",             false);
>  pref("browser.urlbar.userMadeSearchSuggestionsChoice", false);
> +// 4 here means the suggestion notification will be automatically
> +// hidden the 4th day, so it will actually be shown on 3 different days.
> +pref("browser.urlbar.daysBeforeHidingSuggestionsPrompt", 4);

Would be great if you could get a confirm from Steven or Nick that 3 days is the right amount of time.

Comment 4

[Florian Quèze [:florian]]

Steven, the patch here will hide the suggestions notification automatically on the 4th different day, which ensures the prompt has been shown at least 3 times (but likely many more) on 3 different days. Is this value correct?

Comment 5

[Stephen Horlander [:shorlander]]

3 days works for me.

Hard to say what the exact right amount of time should be. But I think 3 days is enough of a time window that people who are going to engage with it will do so.

Comment 6

[Florian Quèze [:florian]]

Looks like this patch is breaking at least these 2 tests:
- browser/base/content/test/urlbar/browser_action_keyword.js
- browser/base/content/test/urlbar/browser_bug556061.js

The error message is:
uncaught exception - TypeError: item.adjustSiteIconStart is not a function at _appendCurrentResult@chrome://global/content/bindings/autocomplete.xml:1294:27

Comment 7

[Marco Bonardo [:mak]]

may be the call to _hideSearchSuggestionsNotification, though it's strange... maybe when we invoke it the item doesn't have the binding attached yet?

Comment 8

[Florian Quèze [:florian]]

Attachment: Patch v2

(In reply to Marco Bonardo [::mak] from comment #7)
> may be the call to _hideSearchSuggestionsNotification, though it's
> strange... maybe when we invoke it the item doesn't have the binding
> attached yet?

The _hideSearchSuggestionsNotification call is indeed what's causing the failures. Making this call only when the notification had actually been shown (ie. when the showSearchSuggestionsNotification class is present) seems to be all it takes to get the tests to pass (at least locally).

Comment 9

[Florian Quèze [:florian]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2be98c53c780

Comment 10

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/fx-team/rev/56fc33e6b14eb446916cdf28edee82df4c548082
Bug 1280381 - The prompt to enable search suggestions in the awesomebar panel should not be shown more than a few times, r=mak.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/56fc33e6b14e

Comment 12

[Florian Quèze [:florian]]

Comment on attachment 8766320 [details] [diff] [review]
Patch v2

Approval Request Comment
[Feature/regressing bug #]: bug 959567

[User impact if declined]: a large notification bar at the top of the awesomebar panel is in the users' way forever, as Telemetry data shows users fail to notice and answer it. The reason we want to uplift to 48 is that it's the release where the awesomebar redesign will ship, and having this notification in the way makes the redesign significantly less compelling.

[Describe test coverage new/current, TreeHerder]: no automated test for this specific change (but there's an automated test for the notification itself), I assume QA will verify.

[Risks and why]: acceptable IMO, as the patch is mostly self-contained, and problems would either be related to the notification visibility (wouldn't be a release blocker), or break the urlbar in a way that would be immediately noticed and reported by beta testers.

[String/UUID change made/needed]: none.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8766320 [details] [diff] [review]
Patch v2

Make sense, taking it. I know where you leave in case of regressions ;)

Should be in 48 beta 5

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9f2b365e53bc

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/95a98cd6e305

Comment 16

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

I managed to test this on 
- latest Nightly 50.0a1 (2016-07-04)
- latest Aurora 49.0a2 (2016-07-04)
- 48.0b5 build1 (20160630123429)
using 
- Windows 10 x64
- Mac OS X 10.11
- Ubuntu 14.04 x86
The fix landed and it works fine, but I have one misunderstanding: if setting the computer local time, when returning back to one of those 3 days when the prompt was initially shown, this time the prompt is not displayed anymore. Is this expected?

Comment 17

[Florian Quèze [:florian]]

(In reply to Iulia Cristescu, QA [:IuliaC] from comment #16)

Thanks for verifying!

> The fix landed and it works fine, but I have one misunderstanding: if
> setting the computer local time, when returning back to one of those 3 days
> when the prompt was initially shown, this time the prompt is not displayed
> anymore. Is this expected?

This is expected. The date only matters so that we are sure the prompt has been shown on at least 3 different days. Once we have recorded it has been shown enough times, we don't look at the date anymore.

Comment 18

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

(In reply to Florian Quèze [:florian] [:flo] from comment #17)
 
> This is expected. The date only matters so that we are sure the prompt has
> been shown on at least 3 different days. Once we have recorded it has been
> shown enough times, we don't look at the date anymore.

Thank you for your clarification! 
Based on previous comments, I will set the flags accordingly.