550410 - Provide preference to disable accelerometer.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Doug Turner (:dougt)]

Introduced in Gecko 1.9.2, we expose device orientation.  We should provide a preference to disable this feature.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 2

[Daniel Veditz [:dveditz]]

Comment on attachment 430531 [details] [diff] [review]
patch v.1

> // Initialize default render-mode.
> pref("mozilla.widget.render-mode", -1);
> 
> // Enable/Disable the geolocation API for content
> pref("geo.enabled", true);
> 
>+// Enable/Disable the orientation API for content
>+pref("accelerometer.enabled", true);

I don't feel strongly about it, but better pref namespacing could prevent future clashes. I'm more worried about the generic "geo." than the one you're actually adding here. It seems like both of those belong under the same root. Dunno if that's some dom. or other platform-like name, or generic mozilla. as in the widget case above.

>+    PRBool bvalue;
>+    rv = prefSrv->GetBoolPref("accelerometer.enabled", &bvalue);
>+    if (NS_SUCCEEDED(rv) && bvalue == PR_FALSE)
>+      mEnabled = PR_FALSE;
>   }

Slightly simpler to do

   rv = prefSrv->GetBoolPref("accelerometer.enabled", &bvalue);
   if (NS_SUCCEEDED(rv))
     mEnabled = bvalue;

or even more middleman removal (at the cost of readability):

    if (NS_SUCCEEDED(prefSrv->GetBoolPref("accelerometer.enabled". &bvalue)))
      mEnabled = bvalue;

I guess you're not in code where you can use nsContentUtils::GetBoolPref()?

    mEnabled = nsContentUtils::GetBoolPref("accelerometer.enabled", PR_TRUE);

All in the nature of nits, change them or not if you wish. r=dveditz

Comment 3

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/843e68865dea

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Not blocking, but I bet we'll take the patch. It can haz tests?

Comment 5

[Doug Turner (:dougt)]

no automated tests.  our testing hardware doesn't have accelerometers.  We should add a litmus test though.

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 430531 [details] [diff] [review]
patch v.1

Approved for 1.9.2.3, a=dveditz

Comment 7

[Doug Turner (:dougt)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/45a5b2dfb738