Closed Bug 1095420 Opened 8 years ago Closed 8 years ago

UI Tour should cause a migration to aurora when highlighting Sync

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 36
Tracking Status
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: past, Assigned: past)

References

Details

Attachments

(2 files, 2 obsolete files)

The migrateToDevEdition action is currently only performed when the user clicks on the aurora-specific "Start using Sync…" link in the preferences panel. It would be best to also do it when the SYnc button is highlighted from the UI Tour.
This is what I think is required. I'm testing it now.
Assignee: nobody → past
Status: NEW → ASSIGNED
Discussing this on IRC, Mark suggested that taking the migration path shoudl be the default behavior, as it will end up in the normal singup/signin flow if the pref is not set anyway.
Attachment #8518858 - Flags: review?(mhammond)
Attachment #8518833 - Attachment is obsolete: true
After more discussion Mark suggested that we simplify further and always check the migration pref in about:accounts. This ensures a migration will be attempted both from the link in the preferences panel as well as from the hamburger menu button. The test still passes.
Attachment #8519014 - Flags: review?(mhammond)
Attachment #8518858 - Attachment is obsolete: true
Attachment #8518858 - Flags: review?(mhammond)
Comment on attachment 8519014 [details] [diff] [review]
UI Tour should cause a migration to aurora when highlighting Sync v3

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

::: browser/base/content/aboutaccounts/aboutaccounts.js
@@ +418,5 @@
> +  }).then(() => {
> +    // Reset the pref after migration.
> +    Services.prefs.setBoolPref("identity.fxaccounts.migrateToDevEdition", false);
> +    return true;
> +  });

I think you should have a trailing .then(null, err => {Cu.reportError(...); return false;}) here
Attachment #8519014 - Flags: review?(mhammond) → review+
(In reply to Mark Hammond [:markh] from comment #4)
> I think you should have a trailing .then(null, err => {Cu.reportError(...);
> return false;}) here

Added in the version of the patch that I pushed.
https://hg.mozilla.org/mozilla-central/rev/93a799501309
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Past, do we want this on Aurora?  If so, can you please request uplift?
Flags: needinfo?(past)
Yes, I'll post an aurora-specific patch.
Flags: needinfo?(past)
Attached patch Patch for AuroraSplinter Review
This is the original patch rebased on top of Aurora tip.
Comment on attachment 8520468 [details] [diff] [review]
Patch for Aurora

Approval Request Comment
[Feature/regressing bug #]: tweak to the feature introduced by bug 1079835
[User impact if declined]: it will be very hard to discover the Sync migration for Developer Edition users. This patch lets the UI Tour point straight at it
[Describe test coverage new/current, TBPL]: m-c, fx-team, existing test
[Risks and why]: minor risk for the Sync button, patch was already tested in m-c
[String/UUID change made/needed]: none
Attachment #8520468 - Flags: approval-mozilla-aurora?
Attachment #8520468 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.