Closed Bug 974496 Opened 6 years ago Closed 6 years ago

Enable sync promo banner

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 --- fixed
firefox30 --- verified
fennec 29+ ---

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(1 file)

Now that bug 974195 is done, we could enable the sync promo banner. However, we probably want to wait for bug 961523 before we do this to avoid annoying users.
tracking-fennec: ? → 29+
Depends on: 975423
No longer depends on: 961523
Attached patch patchSplinter Review
Here's a patch. I'll wait until we get a product call to flip this switch before getting review and landing it.
Depends on: 975469
Comment on attachment 8379942 [details] [diff] [review]
patch

Just need a rubber stamp here :)

I'll wait until bug 975423 is reviewed and land these together.
Attachment #8379942 - Flags: review?(bnicholson)
Attachment #8379942 - Flags: review?(bnicholson) → review+
I realized we'll want to keep this disabled for tests, so that we don't have an issue like bug 965358.

I set this pref to false in the default testing profile and I'm giving this a try push:
https://tbpl.mozilla.org/?tree=Try&rev=0b9a8698583b
https://hg.mozilla.org/integration/fx-team/rev/0d65391f3ef0

I also wonder what impact this will have on eideticker...
https://hg.mozilla.org/mozilla-central/rev/0d65391f3ef0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Status: RESOLVED → VERIFIED
Comment on attachment 8379942 [details] [diff] [review]
patch

Note to sheriffs: I'm requesting approval now, but I don't want to uplift this until the follow-up bug fixes have been uplifted.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): sync promo banner (for new sync)
User impact if declined: new sync won't be promoted
Testing completed (on m-c, etc.): landed on m-c 2/23, working on addressing follow-ups
Risk to taking this patch (and alternatives if risky): the main risk is that this banner will appear when we don't intend for it to appear, or that its appearance will be slightly wrong. we're working on addressing the follow-up bugs that have come up on m-c
String or IDL/UUID changes made by this patch: none
Attachment #8379942 - Flags: approval-mozilla-aurora?
Margaret, if I understand correctly, you want us to approve this patch only when bug 975469 is fixed?
Could you n-i us once it is done? thanks
Flags: needinfo?(margaret.leibovic)
(In reply to Sylvestre Ledru [:sylvestre] from comment #7)
> Margaret, if I understand correctly, you want us to approve this patch only
> when bug 975469 is fixed?
> Could you n-i us once it is done? thanks

Sorry I wasn't very clear. I just want to make sure we uplift most of the banner-related bugs (those bugs that block bug 862801 as well as the fixed ones that block this one) before we flip the switch to turn on this promo banner on aurora.

I'm working on uplifting the current set of approved bugs right now, and after that happens, I think I would feel confident enabling this promo banner on aurora. However, I think QA should verify those bugs on aurora first.

So, yes, I can n-i you once all that is done :)
Flags: needinfo?(margaret.leibovic)
Update here: I just uplifted a majority of the home banner patches to aurora, so once QA verifies that those issues are indeed fixed on aurora, I think it would be safe to go ahead and enable the sync promo banner.

Deb, do you think we should go ahead and enable the sync promo banner on aurora? This would also mean that it would be automatically enabled on beta once 29 merges to beta in a few weeks.
Flags: needinfo?(deb)
Margaret: assuming we'll be able to turn it off in the very very remote chance we need to, then enabling it on Aurora is fine.  The more sync testing we get, the better.
Flags: needinfo?(deb)
Comment on attachment 8379942 [details] [diff] [review]
patch

Margaret, thanks! Approving then!
Attachment #8379942 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.