Closed Bug 1279224 Opened 4 years ago Closed 4 years ago

Remove the Sync promotion in the add-on install doorhanger

Categories

(Firefox :: Sync, defect)

defect
Not set

Tracking

()

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

People

(Reporter: jgruen, Unassigned)

References

Details

Attachments

(4 files)

In add-on (Test Pilot) related user testing, we saw the sync promotion cause a lot of confusion. 

If they have not set up Sync, at the bottom of the door hanger appears a panel that promotes Sync with a link to SUMO.

Clicking the link closes the door hanger, and disrupts the add-on's install flow.

We should remove this marketing messaging here. There are less confusing ways to market sync throughout the product.
Is there some setting or condition that makes you get that sync promotion? +1 on removing it too.
(In reply to Andy McKay [:andym] from comment #1)
> Is there some setting or condition that makes you get that sync promotion?
> +1 on removing it too.

I think it's here:

> If they have not set up Sync, at the bottom of the door 
> hanger appears a panel that promotes Sync with a link to SUMO.
There are 2 addon-related promo messages - when Sync isn't configured we show the one attached by John, and if Sync is configured but addon syncing is disabled, we show one that says "You can use your %S account to synchronize add-ons across multiple devices." which (I think) has the same basic flow.

Do we want to kill that one too? (ni? both John and Ryan, but I'll take an answer from anyone ;)
Flags: needinfo?(rfeeley)
Flags: needinfo?(jgruen)
John says kill both and I agree.
Flags: needinfo?(rfeeley)
Flags: needinfo?(jgruen)
We've already killed the promo from the password doorhanger and after killing these 2 only 1 remains - on the "edit bookmark" popup. Do we want to keep that too or shall we just kill it and remove the promo feature entirely?
John said "kill bookmark promo too" - patch forthcoming. Try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=409f1bec8046&selectedJob=22440172
Comment on attachment 8762913 [details]
Bug 1279224 - remove all Sync promotions from doorhangers.

https://reviewboard.mozilla.org/r/59364/#review56592
Attachment #8762913 - Flags: review?(adw) → review+
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/mozilla-inbound/rev/94c2f240dffa
remove all Sync promotions from doorhangers. r=adw
https://hg.mozilla.org/mozilla-central/rev/94c2f240dffa
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
I have successfully reproduced this bug in firefox nightly 50.0a1(2016-06-09) with elementary OS 0.3.2 Freya 64 bit	

I have verified this bug as fixed in Latest nighly 50.0a1
Build ID 	20160622030210
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0

[bugday-20160622]
I have reproduced this bug with Firefox Nightly 50.0a1 (2016-06-09)
Build ID: 20160609130607
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0
in Windows 8.1, 64-bit.

Verified as fixed with Firefox Nightly 50.0a1 (2016-06-22)
Build ID: 20160622030210
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0

[bugday-20160622]
Awesome! Thanks for working on this.
Comment on attachment 8762913 [details]
Bug 1279224 - remove all Sync promotions from doorhangers.

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Per comment 0 - poor UX for testpilot and various other features.
[Describe test coverage new/current, TreeHerder]: Tests pass
[Risks and why]: Low risk, failure will be obvious enough to make it a relatively low-risk early Aurora uplift
[String/UUID change made/needed]: Strings removed.
Attachment #8762913 - Flags: approval-mozilla-aurora?
Which branches are affected? Does this need to go into 48 as well?
Flags: needinfo?(markh)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #15)
> Which branches are affected? Does this need to go into 48 as well?

All branches are affected. It's bad UX as when a user clicks on a promotion for Sync, the entire doorhanger is dismissed and the user is confused about what happened to their addon install or saved password. On one hand we've lived with it for quite some time, but on the other hand I think we should fix these kinds of warts ASAP.

I didn't request 48 uplift in an attempt to be excessively conservative, but I believe UX jgruen would buy you a beer if we took it there :)
Flags: needinfo?(markh)
Sylvestre, some risk here, what do you think about this for beta 5?
Flags: needinfo?(sledru)
Beta 6 is on me now, Mark can you request uplift?
Flags: needinfo?(markh)
Flags: needinfo?(sledru)
Comment on attachment 8762913 [details]
Bug 1279224 - remove all Sync promotions from doorhangers.

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Per comment 0 - poor UX for testpilot and various other features.
[Describe test coverage new/current, TreeHerder]: Tests pass
[Risks and why]: Low risk, failure will be obvious enough to make it a relatively low-risk early Aurora uplift
[String/UUID change made/needed]: Strings removed.
Flags: needinfo?(markh)
Attachment #8762913 - Flags: approval-mozilla-beta?
Comment on attachment 8762913 [details]
Bug 1279224 - remove all Sync promotions from doorhangers.

Let's uplift this for beta 6. We likely want to make sure to test in beta after it lands to see that the doorhangers still work.
Attachment #8762913 - Flags: approval-mozilla-beta?
Attachment #8762913 - Flags: approval-mozilla-beta+
Attachment #8762913 - Flags: approval-mozilla-aurora?
Attachment #8762913 - Flags: approval-mozilla-aurora+
need l10n approval before 
at least i got * File used for localization (browser/locales/en-US/chrome/browser/browser.properties) altered in this changeset *
remote: 
remote: This repository is string frozen. Please request explicit permission from
remote: release managers to break string freeze in your bug.
remote: If you have that explicit permission, denote that by including in
remote: your commit message l10n=...
remote: *************************************************************


during uplift to aurora
Flags: needinfo?(sledru)
Flags: needinfo?(lhenry)
Mark, could you propose a new patch which will keep the string in aurora & beta? This is the usual workflow for l10n. Flod can sign off
Flags: needinfo?(sledru)
Flags: needinfo?(lhenry)
Flags: needinfo?(francesco.lodolo)
(In reply to Sylvestre Ledru [:sylvestre] [PTO until July 18th] from comment #22)
> Mark, could you propose a new patch which will keep the string in aurora &
> beta? This is the usual workflow for l10n. Flod can sign off

Yes please. Removing strings creates unnecessary noise, so it would be ideal to let the change to browser.properties ride the trains.
Flags: needinfo?(francesco.lodolo)
Version of the patch without the string removals. Same approval request as from comment 19:

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Per comment 0 - poor UX for testpilot and various other features.
[Describe test coverage new/current, TreeHerder]: Tests pass
[Risks and why]: Low risk, failure will be obvious enough to make it a relatively low-risk early Aurora uplift
[String/UUID change made/needed]: None in this version.
Flags: needinfo?(lhenry)
Attachment #8767524 - Flags: review+
Attachment #8767524 - Flags: approval-mozilla-beta?
Attachment #8767524 - Flags: approval-mozilla-aurora?
Comment on attachment 8767524 [details] [diff] [review]
Aurora/Beta version of the patch - no string removals

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

Polish an UX behavior. Take it in 48 beta 6 and aurora.
Attachment #8767524 - Flags: approval-mozilla-beta?
Attachment #8767524 - Flags: approval-mozilla-beta+
Attachment #8767524 - Flags: approval-mozilla-aurora?
Attachment #8767524 - Flags: approval-mozilla-aurora+
Flags: needinfo?(lhenry)
https://hg.mozilla.org/releases/mozilla-aurora/rev/a8e8182cf000

but has problems to apply to beta:

grafting 352474:a8e8182cf000 "Bug 1279224 - remove all Sync promotions from doorhangers (aurora/beta version). r=adw, a=lizzard"
merging browser/base/content/browser.css
merging browser/base/content/browser.xul
merging browser/base/content/popup-notifications.inc
merging browser/base/content/urlbarBindings.xml
merging browser/components/nsBrowserGlue.js
merging browser/themes/linux/browser.css
merging browser/themes/osx/browser.css
merging browser/themes/windows/browser-aero.css
merging browser/themes/windows/browser.css
warning: conflicts while merging browser/themes/linux/browser.css! (edit, then use 'hg resolve --mark')
warning: conflicts while merging browser/themes/osx/browser.css! (edit, then use 'hg resolve --mark')
warning: conflicts while merging browser/themes/windows/browser.css! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

mark can you take a look ?
Flags: needinfo?(markh)
Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Per comment 0 - poor UX for testpilot and various other features.
[Describe test coverage new/current, TreeHerder]: Tests pass
[Risks and why]: Low risk, failure will be obvious enough to make it a relatively low-risk early Beta uplift
[String/UUID change made/needed]: None in this version.
Flags: needinfo?(markh)
Attachment #8767790 - Flags: review+
Attachment #8767790 - Flags: approval-mozilla-beta?
Comment on attachment 8767790 [details] [diff] [review]
Beta version of the patch (no strings removed, css rebased)

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

Take it in 48 beta 6
Attachment #8767790 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [good first verify]
I have reproduced this bug with Nightly 50.0a1 (2016-06-09) on Ubuntu 14.04, 64 bit!

The bug's fix is now verified on latest Realease 48.0,Beta 49.0b3, Aurora 50.0a2.

Realease 48.0:
Build ID 	20160728204513
User Agent 	Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:48.0) Gecko/20100101 Firefox/48.0

Beta 49.0b3:
Build ID 	20160811031722
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0

Aurora 50.0a2:
Build ID 	20160812004003
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0

[testday-20160812]
Reproduced this bug in firefox nightly 50.0a1 (2016-06-09) as comment 0 with windows 10(64 bit)

Verified this bug as fixed with latest firefox release 48.0 (Build ID: 20160726073904)
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

latest firefox Beta 49.0b3 (Build ID: 20160811031722)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0

latest firefox aurora 50.0a2 (Build ID: 20160812004003)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0
QA Whiteboard: [good first verify] → [good first verify] [testday-20160812]
Depends on: 1320157
You need to log in before you can comment on or make changes to this bug.