This is the Fennec equivalent of Bug 1205928. We want to add code around [1] that shows a persistent system notification announcing that Old Sync is no longer supported. There's a question of what the message should say (presumably something like Desktop's message), what action should be provided (delete only? delete and sign up for a Firefox Account?), and how long the notification should hang around before just deleting the account. For folks trying to connect to Mozilla's infrastructure, this is nothing but positive. For self-hosted Old Syncers, this stinks, 'cuz the support we have for Old Sync accounts is going away. [1] https://dxr.mozilla.org/mozilla-central/rev/451a185791433bce1a6a894c27f3da60a3119431/mobile/android/base/sync/receivers/UpgradeReceiver.java#37

I think the idea would be to message in 45, with an option to delete and sign-up for a Firefox Account; and then to remove the Android Account type in 46.

Barbara, this sounds like something we should track in Aha!

Nick and Vivek, you can talk among yourselves to figure out which one of you will fix this :) We should get Anthony thinking about the UX here.

Aha card created, thanks for flagging.

I had commented this in the wrong bug (bug 999198), my bad! (In reply to Anthony Lam (:antlam) from comment #7) > Talked to Nick a bit about this in person, > > I think the best solution here is to use a persistent notification that the > Android system offers. > > For that, we'll need an icon (we can use our current firefox 1 tone icon), a > title and a subtitle. > > +------------------------------------------------+ > | | > | +-+ Sign in to continue syncing | > | +-+ Your account is no longer supported | > | | > +------------------------------------------------+

Bug 1220904 : Part 1 - Message Old sync account going away r?nalexander

Comment on attachment 8684479 [details] MozReview Request: Bug 1220904 : Part 1 - Message Old sync account going away r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24579/diff/1-2/

Bug 1220904 : Part 2 - Added flag to hard delete Sync account in 46 r?nalexander

Comment on attachment 8684569 [details] MozReview Request: Bug 1220904 : Part 2 - Added flag to hard delete Sync account in 46 r?nalexander https://reviewboard.mozilla.org/r/24611/#review22329 Good work! First patch has one substantive question and some nits. Second patch, we'll achieve a different way. Try it out? ::: configure.in:4900 (Diff revision 1) > +dnl = Delete Sync Account in Android Sorry to send you on a goose chase, but we'll achieve this a different way: we'll just remove the Account type enitrely. See https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/xml/sync_authenticator.xml#7. Android deletes accounts with non-existent Account types automatically. You could prepare a patch to remove the XML registration of the Sync account authenticator and SyncAdapter, and verify that your Old Sync accounts get terminated. ::: mobile/android/confvars.sh:104 (Diff revision 1) > +if test `echo "$MOZ_APP_VERSION" | sed "s|\(^[0-9]*\).*|\1|"` -ge 46; then We never test version codes like this -- too fragile. We do this kind of thing by committing flags to the different repositories (mozilla-central, mozilla-aurora, mozilla-beta, mozilla-release).

Comment on attachment 8684479 [details] MozReview Request: Bug 1220904 : Part 1 - Message Old sync account going away r=nalexander I reviewed a version of this.

https://reviewboard.mozilla.org/r/24579/#review22325 ::: mobile/android/base/BrowserApp.java:8 (Diff revision 2) > +import android.accounts.AccountManager; Substantive point: explain why you're bouncing to `BrowserApp`. We could delete the account and offer the sign-in entirely from a "background services" `BroadcastReceiver`; the sign-in would then start Gecko, etc. It's functionally similar, but keeps this kind of migration code out of `BrowserApp`, separating concerns. Am I missing something with the `PendingIntent` or activity handling? ::: mobile/android/base/BrowserApp.java:3552 (Diff revision 2) > + if (UpgradeReceiver.SYNC_ACCOUNT_DEPRECATED_INTENT.equals(action)) { Move this constant to `SyncConstants`, with the others. ::: mobile/android/base/BrowserApp.java:3553 (Diff revision 2) > + // Delete deprecated sync account if exists. ... Old Sync account if one exists. ::: mobile/android/base/BrowserApp.java:3555 (Diff revision 2) > + final Account[] accounts = SyncAccounts.syncAccounts(this); This is synchronous, right? I feel this should be guarded against strict mode errors. (No need to go background, I hope.) ::: mobile/android/base/BrowserApp.java:3564 (Diff revision 2) > + intent.putExtra(FxAccountWebFlowActivity.EXTRA_ENDPOINT, FxAccountConstants.ENDPOINT_PREFERENCES); `ENDPOINT_NOTIFICATION` ::: mobile/android/base/locales/en-US/sync_strings.dtd:274 (Diff revision 2) > +<!-- Notification strings --> Maybe add a localization note saying that these are used in a notification and should be short? ::: mobile/android/base/sync/receivers/UpgradeReceiver.java:107 (Diff revision 2) > + * Launch a persistent notification informing deprecated sync account usage. Show a persistent notifiaction telling the user that their Old Sync account is deprecated. ::: mobile/android/base/sync/receivers/UpgradeReceiver.java:111 (Diff revision 2) > + .setSmallIcon(R.drawable.ic_status_logo) nit: indentation. ::: mobile/android/base/sync/receivers/UpgradeReceiver.java:119 (Diff revision 2) > + PendingIntent pendingIntent = PendingIntent.getActivity(context, 0, notificationIntent, PendingIntent.FLAG_UPDATE_CURRENT); I don't know much about `PendingIntent`s. Explain to me why always using Request Code 0 works. ::: mobile/android/base/sync/receivers/UpgradeReceiver.java:122 (Diff revision 2) > + NotificationManager notificationManager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); nit: final everything! ::: mobile/android/services/strings.xml.in:246 (Diff revision 2) > +<!-- Sync Account deprecated notification --> Maybe add a localization note saying that these are used in a notification and should be short?

Comment on attachment 8684479 [details] MozReview Request: Bug 1220904 : Part 1 - Message Old sync account going away r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24579/diff/2-3/

Bug 1220904 : Part 2 - Added flag to hard delete Sync account r?nalexander

Comment on attachment 8684479 [details] MozReview Request: Bug 1220904 : Part 1 - Message Old sync account going away r=nalexander https://reviewboard.mozilla.org/r/24579/#review23397 Solid! nit: Old Sync, Firefox Account everywhere -- including in the commit message. ::: mobile/android/base/sync/receivers/SyncAccountDeletedService.java:50 (Diff revision 3) > + // Delete Old sync account if exists. tiny (!) nit: Old Sync, Firefox Account. ::: mobile/android/base/sync/receivers/SyncAccountDeletedService.java:51 (Diff revision 3) > + if (SyncAccounts.syncAccountsExist(this)) { nit: There's no need to guard, is there? You can just iterate a possibly empty list. ::: mobile/android/base/sync/receivers/SyncAccountDeletedService.java:67 (Diff revision 3) > + // Returning early here, pickled file will be removed by the intent from AccountAuthenticator. nit: pickle. Can you elaborate on the which intent? Explain what you think is the flow that happens here. ::: mobile/android/services/strings.xml.in:246 (Diff revision 3) > +<!-- Sync Account deprecated notification --> nit: Old Sync.

Comment on attachment 8690278 [details] MozReview Request: Bug 1220904 : Part 2 - Added flag to hard delete Sync account r?nalexander https://reviewboard.mozilla.org/r/25827/#review23399 I think this is reversed. ::: configure.in:4877 (Diff revision 1) > +dnl = Delete Sync Account in Android Include Old Sync Account on Android We really don't want to think of this as an action: we're conditionally including a feature. It'll be included by default, until we don't want it anymore. ::: mobile/android/confvars.sh:102 (Diff revision 1) > +# Delete old sync account This is backwards: "Eanble Old Sync account." ::: mobile/android/services/manifests/SyncAndroidManifest_activities.xml.in:69 (Diff revision 1) > +#ifndef MOZ_ANDROID_OLD_SYNC_ACCOUNT This should be #ifdef. You want it to be there if we are supporting the Old Sync account type, right? ::: mobile/android/services/manifests/SyncAndroidManifest_services.xml.in:1 (Diff revision 1) > +#ifndef MOZ_ANDROID_OLD_SYNC_ACCOUNT Ditto: shouldn't this be ifdef?

Comment on attachment 8684479 [details] MozReview Request: Bug 1220904 : Part 1 - Message Old sync account going away r=nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24579/diff/3-4/

Comment on attachment 8690278 [details] MozReview Request: Bug 1220904 : Part 2 - Added flag to hard delete Sync account r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25827/diff/1-2/

https://reviewboard.mozilla.org/r/24579/#review23733 lgtm! ::: mobile/android/base/sync/receivers/SyncAccountDeletedService.java:50 (Diff revisions 3 - 4) > - // Delete Old sync account if exists. > + // Delete old Sync, Firefox account. Hehe -- sorry, I meant "Old Sync" here, and "Firefox Account" (words capitalized) throughout. So ``` // Delete Old Synca ccount if it exists. ```

Comment on attachment 8690278 [details] MozReview Request: Bug 1220904 : Part 2 - Added flag to hard delete Sync account r?nalexander https://reviewboard.mozilla.org/r/25827/#review23735 ::: mobile/android/services/manifests/SyncAndroidManifest_services.xml.in:1 (Diff revision 2) > +#ifdef MOZ_ANDROID_OLD_SYNC_ACCOUNT Since we're making the whole file conditional, let's make it really conditional: #ifdef the inclusion at https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/AndroidManifest.xml.in#503 Digging into this further, we really want to make *all* the Sync specific includes conditional. That means all the `SyncAndroidManifest*` files, but also the Sync resources ``` mobile/android/base/resources/xml/sync_authenticator.xml mobile/android/base/resources/xml/sync_options.xml mobile/android/base/resources/xml/sync_syncadapter.xml ``` My guess is that we don't actually remove the account type until the `sync_authenticator.xml` file is gone. You can verify by checking in the *Android Settings > Accounts > Add Account* list if *Firefox Sync (Deprecated)* is listed. This fits into a larger project of mine about separating the `services` code into an independent project. The build steps moved in https://bugzilla.mozilla.org/show_bug.cgi?id=773050, but the code and the resources haven't moved into `mobile/android/services` yet. We can either file a ticket to move the resources, or we can do it as part of this ticket. If we do it here, let's just move the `sync*` files into `mobile/android/services/res` and add that directory to the list just before or after `resources` at https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/moz.build#864. I'm not sure about https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/manifests/SyncAndroidManifest_activities.xml.in#77. I wonder if that file is in the correct location -- is it Sync specific, or is it FxA specific?

Any update on this?

https://hg.mozilla.org/integration/fx-team/rev/f5f738d1702ec3ecf534bb0c7f7838790cdd6772 Bug 1220904 - Message that Old Sync accounts are deprecated and going away. r=nalexander

I meant to push this before the merge, but I've landed the first patch that messages the problem. We'll do more comprehensive Sync removal in the 46 cycle.

Comment on attachment 8684479 [details] MozReview Request: Bug 1220904 : Part 1 - Message Old sync account going away r=nalexander Approval Request Comment [Feature/regressing bug #]: desire to deprecate Old Sync 1.1 accounts. [User impact if declined]: we either don't message deprecation and hard-delete accounts with no warning, or we delay deprecation yet further. The Sync 1.1 servers are turned off already, so we don't want to delay dperecation. [Describe test coverage new/current, TreeHerder]: I tested manually. [Risks and why]: very low -- the users this impacts already have no Mozilla-hosted servers and have a completely broken experience. They just don't know it. [String/UUID change made/needed]: yes. This adds two strings. I meant to land this before the merge but Mozlando intervened.

Francesco, are you OK with the two new strings? Thanks

(In reply to Sylvestre Ledru [:sylvestre] from comment #25) > Francesco, are you OK with the two new strings? Thanks My understanding from reading the approval request is that we're already broken: how long has this been true? What's the work that is currently planned for 46, and that would be prevented/delayed if we don't land this warning in 45? To be honest I'd prefer not to have strings landing in mozilla-aurora, even if we're quite early in the cycle. If not landing these strings in 45 stops other work, I guess we'll have to take it, and figure out how a patch with strings and a clear target slipped through the cracks.

(In reply to Francesco Lodolo [:flod] from comment #26) > (In reply to Sylvestre Ledru [:sylvestre] from comment #25) > > Francesco, are you OK with the two new strings? Thanks > > My understanding from reading the approval request is that we're already > broken: how long has this been true? What's the work that is currently > planned for 46, and that would be prevented/delayed if we don't land this > warning in 45? I don't think we're broken, since this isn't on mozilla-aurora yet. We don't feel we can hard delete Old Sync accounts without a warning. We want the warning in 45 so that we can delete the accounts in 46. Right now, the warning is only in 46. > To be honest I'd prefer not to have strings landing in mozilla-aurora, even > if we're quite early in the cycle. If not landing these strings in 45 stops > other work, I guess we'll have to take it, and figure out how a patch with > strings and a clear target slipped through the cracks. Simple: Mozlando. This was on my plate and we were all busy :(

(In reply to Nick Alexander :nalexander from comment #27) > I don't think we're broken, since this isn't on mozilla-aurora yet. I was referring to 'the users this impacts already have no Mozilla-hosted servers and have a completely broken experience.'. So, is this message going to be displayed only to users running self-hosted obsolete servers? Or also users still on an obsolete account? > Simple: Mozlando. This was on my plate and we were all busy :( I understand that, I'm wondering if we have (or should have) ways to know there are such bugs. Anyhow, hopefully we'll get past the nightly/aurora/beta hurdles in 2016. At this point let's take the strings, and land them as soon as possible.

(In reply to Francesco Lodolo [:flod] from comment #28) > (In reply to Nick Alexander :nalexander from comment #27) > > I don't think we're broken, since this isn't on mozilla-aurora yet. > > I was referring to 'the users this impacts already have no Mozilla-hosted > servers and have a completely broken experience.'. This was always intended to be the case for a class of Fennec users. To reduce the amount of native UI Fennec team (me!) had to implement, we provided a migration path for Fennec users who were also syncing with a Desktop device. If you only were using Old Sync with a Fennec device (or Fennec devices), you would be abandoned. Since the only way to arrange a Fennec-only set of devices is to abandon a Desktop device, that seems like a reasonable set to abandon. > So, is this message going to be displayed only to users running self-hosted > obsolete servers? Or also users still on an obsolete account? All users of the obsolete account. (Even if they're self-hosted: self-hosted Old Sync users will lose functionality.) > > Simple: Mozlando. This was on my plate and we were all busy :( > > I understand that, I'm wondering if we have (or should have) ways to know > there are such bugs. Anyhow, hopefully we'll get past the > nightly/aurora/beta hurdles in 2016. > > At this point let's take the strings, and land them as soon as possible. Excellent. I'll uplift.

Comment on attachment 8684479 [details] MozReview Request: Bug 1220904 : Part 1 - Message Old sync account going away r=nalexander Let's take that in aurora then. Thanks to you two!

This will be very hard to QA, since Mozilla no longer runs any Old Sync servers. That means it's almost impossible to get yourself into the state where you would see the message about your account being removed. I am inclined to say no QA is required for this feature.