Bug 884921 - navigator.geolocation should never be null. r=dougt
authorGuilherme Gonçalves <ggp@mozilla.com>
Fri, 26 Jul 2013 16:12:12 -0400
changeset 140199 b54402f21eb8a8c1c585a094826f9e473ac8df26
parent 140198 7b60ddc950079d153aa29fc7d2f16d88bbb62d1c
child 140200 999f228c8ef58dd32e3f692d90534ccedb1954b3
push idunknown
push userunknown
push dateunknown
reviewersdougt
bugs884921
milestone25.0a1
Bug 884921 - navigator.geolocation should never be null. r=dougt
dom/base/Navigator.cpp
dom/tests/mochitest/geolocation/Makefile.in
dom/tests/mochitest/geolocation/test_geolocation_is_undefined_when_pref_is_off.html
dom/webidl/Navigator.webidl
--- a/dom/base/Navigator.cpp
+++ b/dom/base/Navigator.cpp
@@ -956,20 +956,16 @@ Navigator::GetDeviceStorages(const nsASt
   nsDOMDeviceStorage::CreateDeviceStoragesFor(mWindow, aType, aStores, false);
 
   mDeviceStorageStores.AppendElements(aStores);
 }
 
 Geolocation*
 Navigator::GetGeolocation(ErrorResult& aRv)
 {
-  if (!Preferences::GetBool("geo.enabled", true)) {
-    return nullptr;
-  }
-
   if (mGeolocation) {
     return mGeolocation;
   }
 
   if (!mWindow || !mWindow->GetOuterWindow() || !mWindow->GetDocShell()) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
--- a/dom/tests/mochitest/geolocation/Makefile.in
+++ b/dom/tests/mochitest/geolocation/Makefile.in
@@ -13,16 +13,17 @@ include $(DEPTH)/config/autoconf.mk
 MOCHITEST_FILES	= \
 		test_allowCurrent.html \
 		test_allowWatch.html \
 		test_cachedPosition.html \
 		test_cancelCurrent.html \
 		test_cancelWatch.html \
 		test_clearWatch.html \
 		test_clearWatch_invalid.html \
+		test_geolocation_is_undefined_when_pref_is_off.html \
 		test_manyCurrentConcurrent.html \
 		test_manyCurrentSerial.html \
 		test_manyWatchConcurrent.html \
 		test_manyWatchSerial.html \
 		test_manyWindows.html \
 		test_optional_api_params.html \
 		test_shutdown.html \
 		test_windowClose.html \
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/geolocation/test_geolocation_is_undefined_when_pref_is_off.html
@@ -0,0 +1,35 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=884921
+-->
+<head>
+  <title>Test for getCurrentPosition </title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="geolocation_common.js"></script>
+
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank"
+href="https://bugzilla.mozilla.org/show_bug.cgi?id=884921">Mozilla Bug 884921</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+SimpleTest.waitForExplicitFinish();
+
+SpecialPowers.pushPrefEnv({set: [["geo.enabled", false]]}, function() {
+  is(navigator.geolocation, undefined);
+  is("geolocation" in navigator, false);
+  SimpleTest.finish();
+});
+
+</script>
+</pre>
+</body>
+</html>
+
--- a/dom/webidl/Navigator.webidl
+++ b/dom/webidl/Navigator.webidl
@@ -89,22 +89,18 @@ partial interface Navigator {
 // http://www.w3.org/TR/tracking-dnt/ sort of
 partial interface Navigator {
   readonly attribute DOMString doNotTrack;
 };
 
 // http://www.w3.org/TR/geolocation-API/#geolocation_interface
 [NoInterfaceObject]
 interface NavigatorGeolocation {
-  // XXXbz This should perhaps be controleld by the "geo.enabled" pref, instead
-  // of checking it in the C++.  Let's not for now to reduce risk.
-  // Also, we violate the spec as a result, since we can return null.  See bug
-  // 884921.
-  [Throws]
-  readonly attribute Geolocation? geolocation;
+  [Throws, Pref="geo.enabled"]
+  readonly attribute Geolocation geolocation;
 };
 Navigator implements NavigatorGeolocation;
 
 // http://www.w3.org/TR/battery-status/#navigatorbattery-interface
 [NoInterfaceObject]
 interface NavigatorBattery {
     // XXXbz Per spec this should be non-nullable, but we return null in
     // torn-down windows.  See bug 884925.