942716 - Add a developer menu in the system app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Since the settings app can be broken and since it is somewhat painful to go into Settings -> Informations -> More Informations -> Developer let's add the menu in the system app if the pref is turned on. It should not hurt as long as any of the options here is revertable and does not have a side effect of breaking something deep in the device (like resetting some data).

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

The current plan I have is to have some options directly into the sleep menu for developers. That will offer a easily accessible UI that is hard to break.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug942716.part1.patch

Alive if you're around I will need this in order to enable Async Pan Zoom for all Gaia apps today and make sure there is a way to opt-out if Settings is broken (and it will be).

I'm planning to add a second patch to add some specific menu items into it. The UX is not great yet but I don't think it matters too much since this is only for developers.

Comment 3

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8337577 [details] [diff] [review]
bug942716.part1.patch

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

quick r+

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug942716.part2.patch

This is the second part of the patch, that starts to add one developer option to enable/disable async pan zoom. More options can be added in the future.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8337586 [details] [diff] [review]
bug942716.part2.patch

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

Pay attention to the callback in loop :)

::: apps/system/js/sleep_menu.js
@@ +54,5 @@
> +    for (var option in this.developerOptions) {
> +      var defaultValue = this.developerOptions[option].value;
> +      var setting = this.developerOptions[option].setting;
> +      SettingsListener.observe(setting, defaultValue, function(value) {
> +        self.developerOptions[option].value = value;

Are you sure 'option' is valid even if we have other option in developerOptions? I don't think so? You need a closure maybe.

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Alive Kuo [:alive][NEEDINFO] from comment #5)
> Comment on attachment 8337586 [details] [diff] [review]
> bug942716.part2.patch
> 
> Review of attachment 8337586 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Pay attention to the callback in loop :)
> 
> ::: apps/system/js/sleep_menu.js
> @@ +54,5 @@
> > +    for (var option in this.developerOptions) {
> > +      var defaultValue = this.developerOptions[option].value;
> > +      var setting = this.developerOptions[option].setting;
> > +      SettingsListener.observe(setting, defaultValue, function(value) {
> > +        self.developerOptions[option].value = value;
> 
> Are you sure 'option' is valid even if we have other option in
> developerOptions? I don't think so? You need a closure maybe.

Thanks. I'm so used to |let| that I have forgotten to pay attention to this!

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

And, you need two new keys in resource file for l10n pass or it may complain.

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8337586 [details] [diff] [review]
bug942716.part2.patch

Carrying r+.

https://github.com/mozilla-b2g/gaia/commit/485fc6826263bc15f8aeb7a5653af2fcc4792152