The default bug view has changed. See this FAQ.

Support concurrent Fennecs accessing different Android Account types

RESOLVED FIXED in mozilla17

Status

Android Background Services
Android Sync
P1
normal
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla17
All
Android
Dependency tree / graph

Firefox Tracking Flags

(firefox16-)

Details

(Whiteboard: [qa+])

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

5 years ago
Blocks: 761206
(Assignee)

Comment 1

5 years ago
Created attachment 644518 [details] [diff] [review]
Patch against m-i

Part 1/2: make Fennec set Android shared account type depending on package.
(Assignee)

Comment 2

5 years ago
Comment on attachment 644518 [details] [diff] [review]
Patch against m-i

Including mfinkle and blassey for optional feedback, just in case there's an unseen issue with making the account type match the (released in the wild) shared user IDs and singing types.  This is a step towards Bug 761206.
Attachment #644518 - Flags: review?(rnewman)
Attachment #644518 - Flags: feedback?(mark.finkle)
Attachment #644518 - Flags: feedback?(blassey.bugs)
(Assignee)

Comment 3

5 years ago
> shared user IDs and singing types.  This is a step towards Bug 761206.

signing keys.
(Assignee)

Comment 4

5 years ago
There are a few places this change will be user facing, and I'd like to broaden the discussion of what this might look like to the user.

1. we will need to differentiate Account types in Accounts & Settings.

I suggest the following labels:

"Firefox Sync" for (official and beta)
"Firefox Sync for Aurora and Nightly" (for Aurora and Nightly)
"Firefox Sync for Developers" (for developer Fennec builds)

We might want Sync iconography that includes the Fx branding too.

The first one is what we have now.  The second one is probably too long.  The third one should never be visible to the public.

2. we will need to provide some consistency between Send Tab activities.

official and beta will share an Account (good) but both will publish a Send Tab Share intent (bad, since there's now duplication in the Share list).  rnewman thinks that users won't differentiate between Send Tab providers, so these should have the same name and icon.  But if we add features to Send Tab in a future version, there will be no way for the user to distinguish the providers.
(Assignee)

Comment 5

5 years ago
Adding ibarlow for UX input.
(In reply to Nick Alexander :nalexander from comment #4)

> The first one is what we have now.  The second one is probably too long. 

Perhaps "Firefox Sync (pre-Beta)"? Or just "Firefox Sync" with a different icon — Aurora, or with a "beta" banner across it?

> official and beta will share an Account (good) but both will publish a Send
> Tab Share intent (bad, since there's now duplication in the Share list). 
> rnewman thinks that users won't differentiate between Send Tab providers, so
> these should have the same name and icon.

Kinda, and unless there are two of them.

The main issue is that if we customize the name for that activity to avoid confusion, it'll *also* apply when there's only one app involved, which looks janky.

So I would suggest:

* Per-app icon, matching the app
* Same name
* Attempt to filter for the current running Firefox activity, if we're sharing from inside Firefox.
Comment on attachment 644518 [details] [diff] [review]
Patch against m-i

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

::: mobile/android/base/Makefile.in
@@ +232,5 @@
>  ifeq ($(MOZ_APP_NAME),fennec)
>  ICON_PATH = $(topsrcdir)/$(MOZ_BRANDING_DIRECTORY)/content/fennec_48x48.png
>  ICON_PATH_HDPI = $(topsrcdir)/$(MOZ_BRANDING_DIRECTORY)/content/fennec_72x72.png
> +DEFINES += -DMOZ_ANDROID_SHARED_ID="$(ANDROID_PACKAGE_NAME).sharedID"
> +DEFINES += -DMOZ_ANDROID_SHARED_ACCOUNT_TYPE="$(ANDROID_PACKAGE_NAME)_sync"

This will add MOZ_ANDROID_SHARED_ACCOUNT_TYPE to DEFINES twice: once if the app name is "fennec", and once in the clause below. It just so happens that they're the same computed value, but I'd rather be neater about this, because copypasta is evil.
Attachment #644518 - Flags: review?(rnewman) → review+
Comment on attachment 644518 [details] [diff] [review]
Patch against m-i

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

::: mobile/android/base/Makefile.in
@@ +232,5 @@
>  ifeq ($(MOZ_APP_NAME),fennec)
>  ICON_PATH = $(topsrcdir)/$(MOZ_BRANDING_DIRECTORY)/content/fennec_48x48.png
>  ICON_PATH_HDPI = $(topsrcdir)/$(MOZ_BRANDING_DIRECTORY)/content/fennec_72x72.png
> +DEFINES += -DMOZ_ANDROID_SHARED_ID="$(ANDROID_PACKAGE_NAME).sharedID"
> +DEFINES += -DMOZ_ANDROID_SHARED_ACCOUNT_TYPE="$(ANDROID_PACKAGE_NAME)_sync"

one way to fix this is to set a MOZ_ANDROID_SHARED_ACCOUNT_TYPE variable in this logic and then below it do:

ifdef MOZ_ANDROID_SHARED_ACCOUNT_TYPE
DEFINES += -DMOZ_ANDROID_SHARED_ACCOUNT_TYPE="$(MOZ_ANDROID_SHARED_ACCOUNT_TYPE)"
endif
Attachment #644518 - Flags: feedback?(blassey.bugs) → feedback+
(Assignee)

Comment 9

5 years ago
Adding Sean Martell for input on branding.
(Assignee)

Comment 10

5 years ago
Created attachment 645644 [details] [diff] [review]
patch against m-i

Updated to incorporate blassey's feedback.
Attachment #644518 - Attachment is obsolete: true
Attachment #644518 - Flags: feedback?(mark.finkle)
(Assignee)

Comment 11

5 years ago
Depends on Bug 764867 for branding, so that you can distinguish the various Accounts and Send Tab providers.
Depends on: 764867
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/28106e179603
https://hg.mozilla.org/integration/mozilla-inbound/rev/6010529ba8fb
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla17
(Assignee)

Comment 13

5 years ago
To verify:

This ticket lets you concurrently use Sync with one Firefox from each of the following sets:

{official, beta}
{aurora, nightly}
{developer}

1. install official or beta (from Google Play)
2. install Nightly
3. navigate to Android > Settings > Accounts & sync > Add account
4. verify that there are two "Firefox Sync" accounts with *different* icons: one should be the old "Gray Rotating Arrows", one should be the Nightly logo.
5. select the Nightly logo one.
6. pair Nightly with existing Sync account.
7. verify selecting the Nightly button (instead of Settings) takes you to Nightly.
8. verify sync worked in Nightly (history, bookmarks, etc).
9. verify logs like

I FxSync(13050)               fennec :: SyncAdapter :: Syncing account named nalexander+test0724@mozilla.com for client named 'Fennec ncalexan on GT-I9100' with client guid mPRy-KJGydpS (sync account has 0 clients).

10. start Nightly again, and verify "Set up Sync" button IS NOT shown.
11. start Official, and verify "Set up Sync" button IS shown.
12. select "Set up Sync".
13. pair Official with existing Sync account (different than above to test there's no accidental overlap, if possible, but the same account should work fine).
14. verify sync worked in Official (history, etc).
15. verify logs like

I SyncAdapter(13318)          Got onPerformSync. Extras bundle is Bundle[{}]
I SyncAdapter(13318)          Account name: nalexander+test0724@mozilla.com

16. verify that you can share to both "Firefox Sync" options, and the pages get delivered to the correct Sync accounts.
Whiteboard: [qa+]
(Assignee)

Updated

5 years ago
Blocks: 777800
https://hg.mozilla.org/mozilla-central/rev/28106e179603
https://hg.mozilla.org/mozilla-central/rev/6010529ba8fb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Blocks: 755638

Comment 15

5 years ago
Bug 755638 says that it fixed the crashes there, and we're still seeing quite a number of those on Aurora 16, so I think we should track and land this for that train as well.
tracking-firefox16: --- → ?
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #15)
> Bug 755638 says that it fixed the crashes there, and we're still seeing
> quite a number of those on Aurora 16, so I think we should track and land
> this for that train as well.

Apparently all of our crash-reporting users had a Nightly or developer build installed.

This change as landed -- that is, on Nightly but nowhere else -- puts Nightly and Aurora in different sets. If we were to land it on Aurora, Nightly and Aurora would end up back in the same set (see Comment 13), and the crash would reoccur.

The actual fix for the crash is Bug 777800, which is part of the full multi-version support in Bug 761206.

I would like to see this code in Aurora, but not rushed or incomplete.
tracking-firefox16: ? → +
With comment 16 I'm untracking this for 16 then.  We're about a week away from merge and 16 will become Beta, so when the time comes nominate for Aurora as needed but it may not be 16.
tracking-firefox16: + → -
Blocks: 757121
Blocks: 755437
Component: Android Sync → Android Sync
Product: Mozilla Services → Android Background Services
Duplicate of this bug: 768467
You need to log in before you can comment on or make changes to this bug.