Closed Bug 1257901 Opened 4 years ago Closed 4 years ago

Remove "Old Sync" references from empty Synced Tabs panel

Categories

(Firefox for Android :: Android Sync, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: nalexander, Assigned: narek_babajanyan, Mentored)

References

Details

(Whiteboard: [lang=java][good next bug])

Attachments

(2 files, 1 obsolete file)

Fennec no longer supports Old Sync accounts -- it only supports Firefox Accounts.  However, we still have a "Using Old Sync?" link in our Synced Tabs panel state when we have no Firefox Account.  This ticket tracks removing the link and associated code.

The string is https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/remote_tabs_setup.xml#39.  Remove the string from the layout; remove the string from strings.xml.in and sync_strings.dtd; and then remove the ID and click handlers.  See https://dxr.mozilla.org/mozilla-central/search?tree=mozilla-central&q=old_sync%20path%3Amobile&redirect=true.
Mentor: nalexander
Whiteboard: [lang=java][good next bug]
Attachment #8732510 - Flags: review?(nalexander)
Attachment #8732510 - Flags: feedback?(nalexander)
Attached patch oldsync.patchSplinter Review
Attachment #8732510 - Attachment is obsolete: true
Attachment #8732510 - Flags: review?(nalexander)
Attachment #8732510 - Flags: feedback?(nalexander)
Attachment #8732554 - Flags: review?(nalexander)
Attachment #8732554 - Flags: feedback?(nalexander)
Comment on attachment 8732554 [details] [diff] [review]
oldsync.patch

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

Patch is looking good.  Please remove the string declaration and flag me for re-review.

Thanks, Narek!

::: mobile/android/services/src/main/java/org/mozilla/gecko/fxa/FirefoxAccounts.java
@@ -230,5 @@
> -  public static String getOldSyncUpgradeURL(final Resources res, final Locale locale) {
> -    final String VERSION = AppConstants.MOZ_APP_VERSION;
> -    final String OS = AppConstants.OS_TARGET;
> -    final String LOCALE = Locales.getLanguageTag(locale);
> -    return res.getString(R.string.fxaccount_link_old_firefox, VERSION, OS, LOCALE);

There is a string declaration too: https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/strings.xml.in#25 (and the comment above it).
Attachment #8732554 - Flags: review?(nalexander)
Attachment #8732554 - Flags: feedback?(nalexander)
Attachment #8732554 - Flags: feedback+
Comment on attachment 8732651 [details] [diff] [review]
This one does not include the changes of the previous patch

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

Looks good!  I will fold these down into patch and land them.  Great work!

Narek, I don't have a lot of great Fennec mentor tickets right now.  Would you be interested in Bug 1251053?  It requires a little experimentation.
Attachment #8732651 - Flags: review?(nalexander) → review+
Assignee: nobody → narek_babajanyan
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/2bcffdc79a71
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.