964390 - Add pref to disable firefox accounts UI bits

Status: RESOLVED DUPLICATE of bug 949051

(Firefox OS Graveyard :: Gaia, defect)

Description

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

From a gaia perspective, this pref would, at a minimum, prevent users from getting into the fxa flow:
  * disable the firefox accounts screen in the FTE app
  * remove firefox accounts from the settings app

It's a bit more work, but we could also disable other gecko bits, in interest of reducing memory footprint:
  * disable loading the FxAccountsIACHelper (in gaia/shared/js/fxa_iac_client.js)
  * possibly other fxa-related code in gaia

ferjm - ideas on what the pref should be called? it seems like preventing users from hitting the flow is more important than globally disabling helpers, but I could see an argument for going all the way, in case fxa doesn't land in 1.4. Thoughts?

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Jared Hirsch [:_6a68] from comment #0)
> From a gaia perspective, this pref would, at a minimum, prevent users from
> getting into the fxa flow:
>   * disable the firefox accounts screen in the FTE app
>   * remove firefox accounts from the settings app
> 
> It's a bit more work, but we could also disable other gecko bits, in
> interest of reducing memory footprint:
>   * disable loading the FxAccountsIACHelper (in
> gaia/shared/js/fxa_iac_client.js)
>   * possibly other fxa-related code in gaia
> 

Ideally we should have a build flag to disable all fxa related code (Gecko and Gaia sides). Some partners might not want it in their builds or we might not feel confident enough to ship it in v1.4. However, I am ok having only a way to enable/disable the visual parts of fxa. In the end, we can just backout all fxa patches if we get a no-go call for v1.4.

> ferjm - ideas on what the pref should be called?

fxaccounts.enabled maybe?

Yuren, have you got any preference about how to enable/disable features in Gaia?

Comment 2

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

After looking a bit, it appears[1] that we can only enable/disable UI in gaia via a build-time pref. Jed's suggestion in [1] is not to waste the time for a feature that's committed for 1.4, and I'm inclined to agree, especially given that we're quite far along on the committed portions of Settings and FTU integration, while twiddling build params would be a new learning curve all its own.

I'm closing as wontfix. Reopen if we need to discuss further.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=936487#c6

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

We can use a setting to enable/disable FxA. Check [1] for a list of default settings. We can also map this setting to the one for the RP API, check [2].

The code will be there, but at least the visual bits won't be shown.

Feel free to assign this bug to me if needed.

[1] https://mxr.mozilla.org/gaia/source/build/config/common-settings.json
[2] https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/settings.js

Comment 4

[Sam Penrose]

Fernando, that's awesome *if and only if* we can do it quickly. I'm going to assign it to you, but it's only worth doing if it gets the Settings UI landed in the next few days without taking more than a few hours away from your work on forceAuth. We have to have all the new API Gecko code, including https://bugzilla.mozilla.org/show_bug.cgi?id=938635, in the next 3 weeks. Please contact me if I am not making sense, otherwise I will let you decide whether the work is worth it.

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

Attachment: FTU

This patch allow us to disable ftu steps based on a specific boolean setting.

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

The above patch is based on https://github.com/6a68/gaia/tree/bug-949063-add-fxa-to-fte

For settings we can follow the same path as bug 952312. We'll need to wait for it to land or just take the relevant code for our needs.

Jared, do you want to add this to your PRs for ftu (the attached patch) and settings (the same strategy as bug 952312 when it lands) or do you want me to keep it as separate patches? Whatever works the best for you.

Comment 7

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

ferjm: o hai, very nice work! I hadn't thought of using data-attrs to toggle visibility, this is slick!

Do we need tests that this functionality actually toggles the UI off?

For FTU, the simplest thing is to push your commit to github, then I'll cherry-pick & rebase it into the existing squashed commit.

For Settings, the simplest thing is probably to fork my WIP branch[1], add your commit, and issue a PR against my WIP branch (6a68 fork, settings-app-WIP branch), then I"ll merge it in and update the squashed commit that is pending review.

Open to other approaches, that's the first thing that comes to mind for me.

Thanks again for hacking this together, well done :-)

[1] https://github.com/6a68/gaia/tree/settings-app-WIP

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

FTU: https://github.com/ferjm/gaia/commit/4de55c83d5cd62c78365bc0416060c4390f1e185
Settings: https://github.com/6a68/gaia/pull/3

Since this code will land as part of bug 949051 and the FTU bug, I'll mark this one as dup. I have no idea of which bug is tracking the FTU issue anymore, so I am closing this one as dup of bug 949051. Way too many bugs and too many dups... :(

Comment 9

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

ferjm, how about we leave this open and label it as depending on the FTU and settings bugs?

That way, we can be sure it's handled. I have lots of little bugs covered in the gigantic 949051 pull request, we need some way to track them, even if it's just one review that finally gets it all accepted.

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

I'd really prefer to add this as part of bug 949051 (Settings) and bug 897600 (FTU). We already have too many bugs for two single final patches. Once the features land, we can start filling and fixing follow-up bugs related to the features.