889503 - Move Settings API to WebIDL

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Comment 1

[Reuben Morais [:reuben]]

Attachment: Patch

Comment 2

[Reuben Morais [:reuben]]

Attachment: Gaia patch

Comment 3

[David Zbarsky (:dzbarsky)]

Comment on attachment 770325 [details] [diff] [review]
Patch

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

::: dom/settings/SettingsManager.js
@@ -262,5 @@
>  };
>  
> -const SETTINGSMANAGER_CONTRACTID = "@mozilla.org/settingsManager;1";
> -const SETTINGSMANAGER_CID        = Components.ID("{c40b1c70-00fb-11e2-a21f-0800200c9a66}");
> -const nsIDOMSettingsManager      = Ci.nsIDOMSettingsManager;

It may be worth keeping these around.

Comment 4

[Reuben Morais [:reuben]]

Attachment: Patch, v2

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 772792 [details] [diff] [review]
Patch, v2

r=me

Comment 6

[Gregor Wagner [:gwagner]]

Comment on attachment 772792 [details] [diff] [review]
Patch, v2

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

::: dom/settings/SettingsManager.js
@@ +361,5 @@
>      this.hasWritePrivileges = writePerm == Ci.nsIPermissionManager.ALLOW_ACTION;
>  
> +    if (this.hasReadPrivileges) {
> +      cpmm.sendAsyncMessage("Settings:RegisterForMessages");
> +    }

Huh? This defeats the purpose of the whole register message approach. We don't want to get messages if we don't have a listener. This means we have to wake up all apps when a setting has changed. Not good for mobile!

Comment 7

[Reuben Morais [:reuben]]

Attachment: Additional bits outside of dom/settings

Comment 8

[Gregor Wagner [:gwagner]]

Comment on attachment 772792 [details] [diff] [review]
Patch, v2

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

Lets hope the navigator.language issue is getting fixed soon.

Comment 9

[Reuben Morais [:reuben]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/70f4d118d159

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/70f4d118d159
https://hg.mozilla.org/mozilla-central/rev/6c3396af4b17

Comment 11

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/70f4d118d159 broke many gaia-unit tests, see bug 894964.  These tests need to be fixed (or this patch fixed, or backed out) so that we can unhide gaia-unit-tests in TBPL.