Closed Bug 1280381 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox :: Search, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 50
Tracking Status
firefox48 + verified
firefox49 + verified
firefox50 --- verified

People

(Reporter: florian, Assigned: florian)

Details

(Whiteboard: [fxsearch])

Attachments

(1 file, 1 obsolete file)

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.
[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.
I wonder if this needs a test or if this can be covered by QA only.
Attachment #8764329 - Flags: review?(mak77)
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.
Attachment #8764329 - Flags: review?(mak77) → review+
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?
Flags: needinfo?(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.
Flags: needinfo?(shorlander)
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
may be the call to _hideSearchSuggestionsNotification, though it's strange... maybe when we invoke it the item doesn't have the binding attached yet?
Attached patch Patch v2Splinter Review
(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).
Attachment #8764329 - Attachment is obsolete: true
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.
https://hg.mozilla.org/mozilla-central/rev/56fc33e6b14e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
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.
Attachment #8766320 - Flags: approval-mozilla-beta?
Attachment #8766320 - Flags: approval-mozilla-aurora?
Flags: qe-verify+
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
Attachment #8766320 - Flags: approval-mozilla-beta?
Attachment #8766320 - Flags: approval-mozilla-beta+
Attachment #8766320 - Flags: approval-mozilla-aurora?
Attachment #8766320 - Flags: approval-mozilla-aurora+
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?
Flags: needinfo?(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.
Flags: needinfo?(florian)
(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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.