Closed Bug 1270641 Opened 5 years ago Closed 4 years ago

Choose winner of expanded-firstrun and uplift through 47

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox47 --- wontfix
firefox48 --- verified
firefox49 --- unaffected
fennec 48+ ---

People

(Reporter: liuche, Assigned: liuche)

References

Details

Attachments

(1 file, 1 obsolete file)

We need to pick a first run (A, B, C) as the winner and uplift that through Beta.
Barbara, let me know when we'll have an idea about this one, or when we'll have enough data from beta to make this decision.
Flags: needinfo?(bbermes)
tracking-fennec: --- → 47+
Alex, do you know which version won? Was it the one without the Data Saver panel?
Flags: needinfo?(adavis)
Are you referring to this onboarding test?
https://mana.mozilla.org/wiki/pages/viewpage.action?pageId=57508096

Barbara logged her results but didn't formulate a conclusion in the document so I am unsure what she decided to do moving forward.

Version B demonstrated a slight improvement but I don't know if she felt it was worth rolling out.
Flags: needinfo?(adavis)
Barbara, what do you think? Let me know if you have a timeline for when we can pick one of these! If it's long, we might want to just land bug 1268647 with Version B, and then decide later (or Version C, either is fine with me).
Up until today, I thought B would be a slight winner, however looking at the latest results, C is showing some success now too: https://sql.telemetry.mozilla.org/dashboard/experiment-onboarding-with-multiple-slides.

When do you need this decision by? I'm also fine with going with B for now if that's easier, but wanted to bring this up.

ADavis, what are you saying?
Flags: needinfo?(bbermes) → needinfo?(adavis)
Okay, thanks! In that case, if we won't know for a while, I'll just go with C - it'll be easier to remove code rather than add it back in. I'll just land things today, and start working on the uplift patch for getting en_US only in 48 (bug 1268647).
To be clear, I'm not landing anything for this bug, but am rather talking about bug 1268647.
In the dashboard I made, B & C were tied as winners for best retention.

C works for me.
Flags: needinfo?(adavis)
Assignee: nobody → liuche
Attachment #8757028 - Flags: review?(ahunt) → review+
Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

https://reviewboard.mozilla.org/r/55546/#review52312

::: mobile/android/base/java/org/mozilla/gecko/util/Experiments.java:91
(Diff revision 1)
> -        if (SwitchBoard.isInBucket(context, 0, 33)) {
> -            return Experiments.ONBOARDING2_A.equals(experiment);
> -        } else if (SwitchBoard.isInBucket(context, 33, 66)) {
> -            return Experiments.ONBOARDING2_B.equals(experiment);
> -        } else if (SwitchBoard.isInBucket(context, 66, 100)) {
> -            return Experiments.ONBOARDING2_C.equals(experiment);
> +        return Experiments.ONBOARDING2_C.equals(experiment);

Do we need to keep this at all? It looks like we removed the check in FirstrunPagerConfig.getDefault.
(In reply to Andrzej Hunt :ahunt from comment #10)
> Comment on attachment 8757028 [details]
> MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt
> 
> https://reviewboard.mozilla.org/r/55546/#review52312
> 
> ::: mobile/android/base/java/org/mozilla/gecko/util/Experiments.java:91
> (Diff revision 1)
> > -        if (SwitchBoard.isInBucket(context, 0, 33)) {
> > -            return Experiments.ONBOARDING2_A.equals(experiment);
> > -        } else if (SwitchBoard.isInBucket(context, 33, 66)) {
> > -            return Experiments.ONBOARDING2_B.equals(experiment);
> > -        } else if (SwitchBoard.isInBucket(context, 66, 100)) {
> > -            return Experiments.ONBOARDING2_C.equals(experiment);
> > +        return Experiments.ONBOARDING2_C.equals(experiment);
> 
> Do we need to keep this at all? It looks like we removed the check in
> FirstrunPagerConfig.getDefault.

It's still being used by the Kinto downloadable content (incorrectly, I think) but I commented on that to see if it was intended to be a live Switchboard query, or perhaps it's meant to be added to this call.

Aurora try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f73acc49f46
Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

Approval Request Comment
[Feature/regressing bug #]: Turn off experiment
[User impact if declined]: Users will still be in an experiment
[Describe test coverage new/current, TreeHerder]: local, try runs
[Risks and why]: Low, removing code
[String/UUID change made/needed]: removal of unused strings, no need to re-localize
Attachment #8757028 - Flags: approval-mozilla-beta?
Attachment #8757028 - Flags: approval-mozilla-aurora?
Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

I spoke to Margaret and mentioned the fact that RC week is only for release blocking issues. Keeping that bar in mind, we may not have enough time to stop this experiment. Let's uplift only to Aurora48.
Attachment #8757028 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
tracking-fennec: 47+ → 48+
any reason why this didn't land in nightly?
(In reply to Sylvestre Ledru [:sylvestre] from comment #14)
> any reason why this didn't land in nightly?

The first run flow is different on 49+ (bug 1268641), so this bug didn't affect Nightly.

Why has this still not landed on Aurora? At this point this will need to be uplifted to Beta now.
Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

Trying to flag this for beta, because this missed aurora merge, but it looks like I can't clear the b- flag.
Flags: needinfo?(rkothari)
Attachment #8757028 - Flags: approval-mozilla-aurora?
Comment on attachment 8757028 [details]
MozReview Request: Bug 1270641 - Choose winner of expanded-firstrun. r=ahunt

This is approved for uplift to Beta48.
Flags: needinfo?(rkothari)
Attachment #8757028 - Flags: approval-mozilla-beta- → approval-mozilla-beta+
Hi Wes, just fyi, this needs to land on Beta48. Thanks!
Flags: needinfo?(wkocher)
Is it possible to have a patch that doesn't touch localization files for string frozen branches (aurora, beta)? Removing strings from localization files is going to create unnecessary noise in tools and dashboards, so having a patch without changes to android_strings.dtd would be ideal.

The patch removing strings can then ride the trains.

To be honest it's also absolutely unclear why this never landed in nightly and it's jumping directly to beta.
Flags: needinfo?(liuche)
i guess we need to wait till comment#19 is fixed before we can uplift
Flags: needinfo?(wkocher)
Carrying over review flag, removed dtd changes.

> To be honest it's also absolutely unclear why this never landed in nightly
> and it's jumping directly to beta.

Margaret addressed this in Comment #15, and the tracking flags also show that neither 49 nor 50 are affected. Nightly (at the time of the a? request) had a completely different flow, and this didn't get uplifted so now we're landing it on beta.
Attachment #8757028 - Attachment is obsolete: true
Flags: needinfo?(liuche)
Attachment #8761860 - Flags: review+
Ready to land now, I removed the string changes.

Beta try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eff519e8b258
Flags: needinfo?(cbook)
landed! -> https://hg.mozilla.org/releases/mozilla-beta/rev/1510116bd5e5
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(cbook)
Resolution: --- → FIXED
Verified as fixed on latest Beta build, 48.0b2. This fix was tested on a Xiaomi mi i4 with Android 5.0.2 and on a Xiaomi mi Pad2 with Android 5.1.1.
You need to log in before you can comment on or make changes to this bug.