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 140206 b54402f21eb8a8c1c585a094826f9e473ac8df26
parent 140205 7b60ddc950079d153aa29fc7d2f16d88bbb62d1c
child 140207 999f228c8ef58dd32e3f692d90534ccedb1954b3
push id31643
push userryanvm@gmail.com
push dateFri, 26 Jul 2013 20:14:02 +0000
treeherdermozilla-inbound@60542f3605e3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdougt
bugs884921
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.