862353 - Set dom.mozContacts.enabled in mochitests

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Reuben Morais [:reuben]]

We should enable those in the automation instead of adding pref setting code to every test that uses those APIs.

Comment 1

[Reuben Morais [:reuben]]

Attachment: Enable mozContacts and mozSettings for mochitests

Comment 2

[Reuben Morais [:reuben]]

Comment on attachment 737993 [details] [diff] [review]
Enable mozContacts and mozSettings for mochitests

We can't do this right now because the app-startup branch is NavigatorPropertyHelper runs too early. I plan on implementing this in a way that will respect resetting the pref later (and having the pref enabled on the profile).

Ben, you can use addLoadEvent in the test to fix bug 778256 until this is done, if you want.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Thanks Reuben.  I'll check it out tonight.  I'm curious if this will address the original issue on Android in bug 778256.  We'll see what the try server says.

Comment 4

[Reuben Morais [:reuben]]

Attachment: Enable dom.mozContacts.enabled in mochitests

This is based in bug 863704, which just landed. Settings is more complicated, I'm looking into it.

Comment 5

[Reuben Morais [:reuben]]

Comment on attachment 744381 [details] [diff] [review]
Enable dom.mozContacts.enabled in mochitests

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

::: testing/profiles/prefs_general.js
@@ +110,5 @@
>  user_pref("layout.css.report_errors", true);
> +
> +// Enable mozContacts
> +user_pref("dom.navigator-property.disable.mozContacts", false);
> +user_pref("dom.global-constructor.disable.mozContact", false);

This is missing the main "dom.mozContacts.enabled" pref for some reason, pretend that it's here :)

Comment 6

[Reuben Morais [:reuben]]

Attachment: Set dom.mozSettings.enabled to true in tests

The settings problem might be simpler than I originally thought.

Comment 7

[Reuben Morais [:reuben]]

Attachment: Remove NavigatorPropertyHelper

Comment 8

[Reuben Morais [:reuben]]

Let's see if the settings patch continues to misbehave:
https://tbpl.mozilla.org/?tree=Try&rev=1bbe3696c921

Comment 9

[Reuben Morais [:reuben]]

Attachment: Enable mozContacts in B2G and mochitests

This allows us to do a bunch of clean up on the tests and remove the NavigatorPropertyHelper service, which serves the sole purpose of letting desktop mochitests access the API. Ben, does this look OK?

Comment 10

[Reuben Morais [:reuben]]

Attachment: Enable mozSettings in B2G and mochitests

I think I figured the VM mochitest failures out, let's see: https://tbpl.mozilla.org/?tree=Try&rev=5d7714241d26

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 746017 [details] [diff] [review]
Enable mozContacts in B2G and mochitests

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

Sounds fine to me!

Comment 12

[Gregor Wagner [:gwagner]]

Comment on attachment 746017 [details] [diff] [review]
Enable mozContacts in B2G and mochitests

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

::: b2g/app/b2g.js
@@ +384,5 @@
>  
>  // Temporary permission hack for WebContacts
>  pref("dom.mozContacts.enabled", true);
> +pref("dom.navigator-property.disable.mozContacts", false);
> +pref("dom.global-constructor.disable.mozContact", false);

Alright if we have 3 redundant prefs we should unify them. Either all of them have to be true or false.

Comment 13

[Reuben Morais [:reuben]]

Comment on attachment 746020 [details] [diff] [review]
Enable mozSettings in B2G and mochitests

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

Same thing but for Settings. Also fixes some geolocation tests that flip the pref.
Ben, does this fix the Android failures?

Comment 14

[Reuben Morais [:reuben]]

Thanks!

https://hg.mozilla.org/projects/birch/rev/74b0d6784412
https://hg.mozilla.org/projects/birch/rev/62b448f166ad
https://hg.mozilla.org/projects/birch/rev/97b938ea14fe

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/74b0d6784412
https://hg.mozilla.org/mozilla-central/rev/62b448f166ad
https://hg.mozilla.org/mozilla-central/rev/97b938ea14fe

Comment 16

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Remove stale code in geolocation that checks for mozsettings.

As discussed on IRC, this check in geolocation_common.js should not be needed any more.

https://tbpl.mozilla.org/?tree=Try&rev=ffd7c26ea262

Comment 17

[Reuben Morais [:reuben]]

Comment on attachment 749374 [details] [diff] [review]
Remove stale code in geolocation that checks for mozsettings.

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

Nice catch, thanks!

::: dom/tests/mochitest/geolocation/geolocation_common.js
@@ +97,5 @@
>    // optional  ok(location.coords.altitudeAccuracy == 42, "alt acc matches known value");
>  }
>  
>  function toggleGeolocationSetting(value, callback) {
>    var mozSettings = window.navigator.mozSettings;

nit: inline this into the next line

Comment 18

[Reuben Morais [:reuben]]

https://hg.mozilla.org/projects/birch/rev/9a69db5596ce

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9a69db5596ce

Comment 20

[Philip Chee]

Thunderbird bustage fix:
http://hg.mozilla.org/comm-central/rev/7c9d7c3ebc5c