1113415 - Instructions for enabling syncing not precise in Android 5.0

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Android Sync, defect)

Description

[Vladimir Rutsky]

Attachment: "Auto-sync data" option screenshot in Settings -> Accounts

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0
Build ID: 20141127111021

Steps to reproduce:

1. Configure syncing of Firefox on Android: Menu -> Settings -> Sync -> login using existing account.
2. Try to setup what should be synced when global sync on Android is disabled. 


Actual results:

Firefox will display warning message about disabled global sync:

Sync in set up, but not syncing automatically. Toggle "Auto-sync data" in Android Settings > Data Usage.

These instructions are incorrect on Android 5: there is no such option "Auto-sync data" in Data Usage page (or subpages).


Expected results:

Firefox should display proper instructions on Android 5: global syncing can be enabled on Android 5 in Settings -> Accounts -> "Auto-sync data" checkbox in Menu.

Tested on Nexus 5 with Android 5.0.

Comment 1

[Aaron Train [:aaronmt]]

Confirmed.

Comment 2

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

Great bug report, thanks!

This is a good first or a great second bug, but you will probably want an Android 5+ device for testing.  (Not strictly needed, but helpful.)

What needs to happen is that we need two versions of the string [1] (and in strings.xml.in at [2]).  We could do this with fancy per-Android-version strings.xml files, but that's really difficult to localize, so instead add a copy of the string with -v21 in the name and at [3], check the version (using Versions.*) and set the preference title to the appropriate string.

[1] http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/locales/en-US/sync_strings.dtd#208

[2] http://dxr.mozilla.org/mozilla-central/source/mobile/android/services/strings.xml.in?from=services/strings.xml.in&case=true#198

[3] http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/activities/FxAccountStatusFragment.java#336

Comment 3

[happygrape2]

Hi! I'm new to this and would like to take this as my first bug. I have some Java experience and some introductory Android experience. I don't have an Android device, though. Could I be assigned to this bug?

Comment 4

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

(In reply to happygrape2 from comment #3)
> Hi! I'm new to this and would like to take this as my first bug. I have some
> Java experience and some introductory Android experience. I don't have an
> Android device, though. Could I be assigned to this bug?

Hey!  We've been talking about the best "assign bug" process for our community over at [1].  I generally don't assign bugs before seeing some progress (usually, a patch) on the ticket.  This isn't a big deal, we just don't want to block other contributors from tickets prematurely.

You can work on this with an Android emulator, but it's not 100% easy to get emulators up and running.  Your first steps will definitely be getting a Fennec build completed [2] and getting your build running on an emulator.  Let us know how you progress.

[1] https://mail.mozilla.org/pipermail/mobile-firefox-dev/2014-December/001031.html

[2] https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec

Comment 5

[Prabhjyot Sodhi [:psd]]

I would love to work on this bug if nobody else is :)
I have my fennec build ready and I have a device running android 4.4.

Comment 6

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

(In reply to Prabhjyot Sodhi [:psd] from comment #5)
> I would love to work on this bug if nobody else is :)
> I have my fennec build ready and I have a device running android 4.4.

Do it!  As I said in the previous comment, we're trying to assign bugs after the first things get posted to the ticket.

Comment 7

[Prabhjyot Sodhi [:psd]]

Hi!

As I have understood, I have a add the following new line to [1] :

<!ENTITY fxaccount_status_needs_master_sync_automatically_enabled_v21 '&syncBrand.shortName.label; is set up, but not syncing automatically. Toggle “Auto-sync data” in Android Settings &gt; Accounts &gt; Data Usage.'>

and add the following line to [2]

<string name="fxaccount_status_needs_master_sync_automatically_enabled_v21">&fxaccount_status_needs_master_sync_automatically_enabled_v21;</string>

But I did not understand the 'checking the version using Version.*' part.
I understood the need of the doing this but didn't get how you want me to do it.
Thanks

Comment 8

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

(In reply to Prabhjyot Sodhi [:psd] from comment #7)
> Hi!
> 
> As I have understood, I have a add the following new line to [1] :
> 
> <!ENTITY fxaccount_status_needs_master_sync_automatically_enabled_v21
> '&syncBrand.shortName.label; is set up, but not syncing automatically.
> Toggle “Auto-sync data” in Android Settings &gt; Accounts &gt; Data Usage.'>
> 
> and add the following line to [2]
> 
> <string
> name="fxaccount_status_needs_master_sync_automatically_enabled_v21">&fxaccoun
> t_status_needs_master_sync_automatically_enabled_v21;</string>

Yep!
 
> But I did not understand the 'checking the version using Version.*' part.

Let's see.  The function showNeedsMasterSyncAutomaticallyEnabled in [3] is responsible for displaying the message to the user when it's appropriate.  What you need to do is set the message depending on the phone's Android version, using Versions from [4].  Something like: (untested):

  protected void showNeedsMasterSyncAutomaticallyEnabled() {
    syncCategory.setTitle(R.string.fxaccount_status_sync);
    needsMasterSyncAutomaticallyEnabledPreference.setTitle(
        Versions.preLollipop ? R.string.NON_V21_STRING : R.string.NEW_V21_STRING);
    showOnlyOneErrorPreference(needsMasterSyncAutomaticallyEnabledPreference);
    setCheckboxesEnabled(false);
  }

[3] http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/activities/FxAccountStatusFragment.java#334

[4] http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/AppConstants.java.in#34

Comment 9

[Vladimir Rutsky]

(In reply to Prabhjyot Sodhi [:psd] from comment #7)
> Hi!
> 
> As I have understood, I have a add the following new line to [1] :
> 
> <!ENTITY fxaccount_status_needs_master_sync_automatically_enabled_v21
> '&syncBrand.shortName.label; is set up, but not syncing automatically.
> Toggle “Auto-sync data” in Android Settings &gt; Accounts &gt; Data Usage.'>

Global sync setting in Android 5 is not in ``Android Settings -> Accounts -> Data Usage``, but in ``Android Settings -> Accounts -> "Auto-sync data" checkbox in the Menu`` — see attached to the bug screenshot.

Comment 10

[Prabhjyot Sodhi [:psd]]

(In reply to Vladimir Rutsky from comment #9)

> Global sync setting in Android 5 is not in ``Android Settings -> Accounts ->
> Data Usage``, but in ``Android Settings -> Accounts -> "Auto-sync data"
> checkbox in the Menu`` — see attached to the bug screenshot.

<!ENTITY fxaccount_status_needs_master_sync_automatically_enabled_v21 '&syncBrand.shortName.label; is set up, but not syncing automatically. Check “Auto-sync data” in the Menu of Android Settings &gt; Accounts.'>

Is this better?

Comment 11

[Vladimir Rutsky]

(In reply to Prabhjyot Sodhi [:psd] from comment #10)
> (In reply to Vladimir Rutsky from comment #9)
> 
> > Global sync setting in Android 5 is not in ``Android Settings -> Accounts ->
> > Data Usage``, but in ``Android Settings -> Accounts -> "Auto-sync data"
> > checkbox in the Menu`` — see attached to the bug screenshot.
> 
> <!ENTITY fxaccount_status_needs_master_sync_automatically_enabled_v21
> '&syncBrand.shortName.label; is set up, but not syncing automatically. Check
> “Auto-sync data” in the Menu of Android Settings &gt; Accounts.'>
> 
> Is this better?

Yes, this is the correct description.

Comment 12

[Vladimir Rutsky]

Offtopic: during posting previous comment I checked "Clear the needinfo request for [e-mail]." checkbox, but still bug has needingo flag.

Comment 13

[Vladimir Rutsky]

Offtopic: Hm, from second attempt needinfo flag was removed...

Comment 14

[Prabhjyot Sodhi [:psd]]

(In reply to Vladimir Rutsky from comment #11)
 
> Yes, this is the correct description.

Awesome :)

Comment 15

[Prabhjyot Sodhi [:psd]]

Hi!
I have understood what is to be done and how I have to go about it.
But I am now having a problem building it again.
I am getting the following output : http://pastebin.mozilla.org/8139252

I have tried setting up my src directory again (i.e re unbundled the .hg file again and ... )
Thanks :)

Comment 16

[Prabhjyot Sodhi [:psd]]

Attachment: 1113415.patch

Got my build sorted out :)
Untested.
How should I test for a android lolipop device?

Comment 17

[Prabhjyot Sodhi [:psd]]

Attachment: 1113415.patch

Made a mistake in the last one.

Comment 18

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

Comment on attachment 8541907 [details] [diff] [review]
1113415.patch

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

Hi psd, this is looking great!  I have a couple of tiny suggestions (nits) that I'd like to see addressed, and you'll need to update the commit message before we can land, like:

Bug 1113415 - Make master sync instructions depend on Android version. r=nalexander

As for testing, if you have a v21 device you should be able to:

1) create a Firefox Account;
2) start Syncing;
3) go to Android Settings > Accounts and toggle the option *off*;
4) in Fennec, open Settings > Sync;
5) verify the newly worded warning is shown.
6) go to Android Settings > Accounts and toggle the option *on*;
7) return to Fennec, open Settings > Sync;
8) verify no warning is shown.

Let me know how that works.  I'll be away for a few days until early January, but flag me for review and we will get this landed then.  Great work!

::: mobile/android/base/fxa/activities/FxAccountStatusFragment.java
@@ +333,5 @@
>    }
>  
>    protected void showNeedsMasterSyncAutomaticallyEnabled() {
>      syncCategory.setTitle(R.string.fxaccount_status_sync);
> +    needsMasterSyncAutomaticallyEnabledPreference.setTitle(AppConstants.Versions.preLollipop ? R.string.fxaccount_status_needs_master_sync_automatically_enabled : R.string.fxaccount_status_needs_master_sync_automatically_enabled_v21);

nit: this line is really long!  Break it like:

...setTitle(... preLollipop ?
    R.string.... :
    R.string....);

::: mobile/android/base/locales/en-US/sync_strings.dtd
@@ +205,5 @@
>  <!ENTITY fxaccount_status_needs_verification2 'Your account needs to be verified. Tap to resend verification email.'>
>  <!ENTITY fxaccount_status_needs_credentials 'Cannot connect. Tap to sign in.'>
>  <!ENTITY fxaccount_status_needs_upgrade 'You need to upgrade &brandShortName; to sign in.'>
>  <!ENTITY fxaccount_status_needs_master_sync_automatically_enabled '&syncBrand.shortName.label; is set up, but not syncing automatically. Toggle “Auto-sync data” in Android Settings &gt; Data Usage.'>
> +<!ENTITY fxaccount_status_needs_master_sync_automatically_enabled_v21 '&syncBrand.shortName.label; is set up, but not syncing automatically. Check “Auto-sync data” in the Menu of Android Settings &gt; Accounts.'>

Let's say "Toggle" instead of "Check", to be consistent with the above.  And lowercase "Menu".

Comment 19

[Vladimir Rutsky]

(In reply to Nick Alexander :nalexander from comment #18)
> Let's say "Toggle" instead of "Check", to be consistent with the above.  And
> lowercase "Menu".

Just FYI I found that in the official Nexus phone docs [1] the "Menu" button is called in uppercase and in the same way as other settings menus, e.g.:

    To control auto-sync for all apps that use it, open *Settings* > *Data usage* > *Menu* and check or uncheck *Auto-sync data*.

(Looks like this is instructions for pre Lolipop Android.)

[1] https://support.google.com/nexus/answer/2840875?hl=en

Comment 20

[Prabhjyot Sodhi [:psd]]

Attachment: 1113415.patch

I have made the changes that you suggested but I don't have a v21 device.

Comment 21

[Vladimir Rutsky]

I can test a build on my Nexus 5 with Android 5 if it will be reproducibly built on trusted system (e.g. on Mozilla Try server).

Comment 22

[Prabhjyot Sodhi [:psd]]

(In reply to Vladimir Rutsky from comment #21)
> I can test a build on my Nexus 5 with Android 5 if it will be reproducibly
> built on trusted system (e.g. on Mozilla Try server).

Awesome!, Let's wait for nalexander for the built on try server part :)

Comment 23

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

ni to me so htis doesn't get lost.

Comment 24

[Prabhjyot Sodhi [:psd]]

Hi!
Happy new year Nick :)
Will do it the next time.

off topic though: Where can I look for upcoming or relatively small mozilla projects. I would love to contribute directly to code along with the bugs. 
Thanks

Comment 25

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

Comment on attachment 8542057 [details] [diff] [review]
1113415.patch

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

This look good!  I have some things to land, I should get to it today.  Bug me if not!

Comment 26

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

https://hg.mozilla.org/integration/fx-team/rev/02d184fdb143

Comment 27

[Nick Alexander :nalexander [he/him] (PTO August 1-10)]

Landed.  Good work, psd!  In answer to your question, start with bugsahoy: http://www.joshmatthews.net/bugsahoy/?mobile=1 for finding good bugs.  You're always welcome to reach out to mentors directly: we don't always have good next steps, but we're trying to get better at building our contributors into area experts.

I personally have a nice second-and-third bug project described: Bug 1112673.  I expect you'd collaborate with me and :vivek.

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/02d184fdb143

Comment 29

[Prabhjyot Sodhi [:psd]]

(In reply to Nick Alexander :nalexander from comment #27)
> I personally have a nice second-and-third bug project described: Bug
> 1112673.  I expect you'd collaborate with me and :vivek.

Awesome! I would love to work on bug 1112673 :)