Last Comment Bug 1313580 - Remove web content access to Battery API
: Remove web content access to Battery API
Status: VERIFIED FIXED
: addon-compat, dev-doc-complete, privacy, site-compat
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: Unspecified Unspecified
-- normal (vote)
: mozilla52
Assigned To: Chris Peterson [:cpeterson]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: http-fingerprint 1314076 678694
  Show dependency treegraph
 
Reported: 2016-10-27 23:28 PDT by Chris Peterson [:cpeterson]
Modified: 2017-01-17 09:56 PST (History)
11 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
verified
52+


Attachments
part-3-make-battery-ChromeOnly.patch (12.68 KB, patch)
2016-10-27 23:28 PDT, Chris Peterson [:cpeterson]
amarchesini: review+
Details | Diff | Splinter Review
part-2-remove-battery-telemetry.patch (2.13 KB, patch)
2016-10-27 23:33 PDT, Chris Peterson [:cpeterson]
amarchesini: review+
Details | Diff | Splinter Review
part-1-remove-b2g-battery-tests.patch (17.92 KB, patch)
2016-10-27 23:33 PDT, Chris Peterson [:cpeterson]
amarchesini: review+
Details | Diff | Splinter Review

Description User image Chris Peterson [:cpeterson] 2016-10-27 23:28:22 PDT
Created attachment 8805443 [details] [diff] [review]
part-3-make-battery-ChromeOnly.patch

Remove web content access to Battery API, as proposed on dev-platform:

https://groups.google.com/d/msg/mozilla.dev.platform/5U8NHoUY-1k/9ybyzQIYCAAJ

I changed the battery mochitests to chrome tests and added a mochitest to verify that navigator.battery and navigator.getBattery are undefined in unprivileged web content. I also added "use strict" to these tests.

I disabled the web-platform battery tests except for the test for the navigator.getBattery() interface, which is now expected to fail.

If the Battery API is restricted to chrome or extensions, perhaps we can remove the rounding and reduced precision that was added in bugs like bug 1292655 to reduce the effectiveness of fingerprinting.

I didn't bother to remove the dom.battery.enabled pref.
Comment 1 User image Chris Peterson [:cpeterson] 2016-10-27 23:33:05 PDT
Created attachment 8805444 [details] [diff] [review]
part-2-remove-battery-telemetry.patch

Part 2: Remove telemetry probe for Battery API.

We don't need to collect telemetry for an API that is no longer web-facing. Also, this probe expires in Firefox 52.
Comment 2 User image Chris Peterson [:cpeterson] 2016-10-27 23:33:30 PDT
Created attachment 8805445 [details] [diff] [review]
part-1-remove-b2g-battery-tests.patch

Part 1: Remove unused B2G tests for Battery API.
Comment 3 User image Andrea Marchesini [:baku] 2016-10-27 23:48:58 PDT
Comment on attachment 8805443 [details] [diff] [review]
part-3-make-battery-ChromeOnly.patch

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

This code is OK, but why do we want to keep th battery API? Can just get rid of it completely?
Which chrome-only code uses it?
Comment 4 User image Andrea Marchesini [:baku] 2016-10-27 23:49:28 PDT
See my last comment.
Comment 5 User image Chris Peterson [:cpeterson] 2016-10-28 09:50:51 PDT
(In reply to Andrea Marchesini [:baku] from comment #3)
> This code is OK, but why do we want to keep th battery API? Can just get rid
> of it completely?
> Which chrome-only code uses it?

No Firefox chrome code uses the battery API. A couple "battery monitor" add-ons use it, but that seems like a pretty niche use case for a browser add-on.

I can remove the battery code completely. Making the API chrome-only just seemed like a less controversial proposal. :) I can submit a follow-up patch to remove the code, if you support that.
Comment 6 User image Andrea Marchesini [:baku] 2016-10-28 10:49:26 PDT
I would like to know if WebExtensions team is interested to expose this feature.
Then we can talk with addon community folks asking them to contact the developers of these battery monitor addons.
But first we must have an alternative to offer.

Maybe just write a comment in the WebIDL file saying why this API is just chrome-only.
Comment 7 User image Pulsebot 2016-10-28 18:46:35 PDT
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f8f02af9d17
Part 1: Remove unused B2G tests for Battery API. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/585f52662d69
Part 2: Remove telemetry probe for Battery API. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/6069dc9afe31
Part 3: Remove web content access to Battery API. r=baku
Comment 8 User image Chris Peterson [:cpeterson] 2016-10-28 22:17:46 PDT
I only found one add-on on AMO that clearly uses the battery API ("Easy Battery Monitor" [1]), but there are a few on DXR [2] ("Battery Logger" and "The Great Suspender") and the Chrome Web Store. So we should probably leave the battery API available to extensions for now. 

[1] https://addons.mozilla.org/en-US/firefox/addon/easy-battery-monitor/
[2] https://dxr.mozilla.org/addons/search?q=regexp%3A%5CbgetBattery%5Cb&redirect=false
Comment 9 User image Andrea Marchesini [:baku] 2016-10-29 00:31:47 PDT
Jorge, we need your help here :)
We would like to get rid of Battery API but, as you can see in Comment 8, there are some addons using  it.
What do you suggest in this case?
Comment 11 User image Kohei Yoshino [:kohei] 2016-10-29 06:23:00 PDT
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/battery-status-api-has-been-removed/
Comment 12 User image Zibi Braniecki [:gandalf][:zibi] 2016-10-29 10:16:01 PDT
Is there a chance that we will want to internally use this API to inform our behavior? I can see a variety of power-saving options that could behave similarly to how tlp works in linux.

Also, in any kiosk mode that Gecko might be used in this API would be crucial to have.

I know it's hypothetical, but I'm wondering if this API is problematic enough to warrant it's removal
Comment 13 User image Andrea Marchesini [:baku] 2016-10-29 13:39:18 PDT
> Is there a chance that we will want to internally use this API to inform our
> behavior? I can see a variety of power-saving options that could behave
> similarly to how tlp works in linux.

We already do this. QuotaManager uses the HAL to detect when it's the right time to do some FileSystem checks.
We are not removing this functionality. Just the DOM layer.
Comment 14 User image Chris Peterson [:cpeterson] 2016-10-29 23:23:33 PDT
Release Note Request (optional, but appreciated)
[Why is this notable]: This bug removes a public DOM API.
[Affects Firefox for Android]: Yes.
[Suggested wording]: "Removed Battery Status API to reduce fingerprinting of users by trackers" (or the boring alternative: "Removed Battery Status API")
[Links (documentation, blog post, etc)]:
Comment 15 User image Lukasz 2016-10-30 02:52:39 PDT
(In reply to Chris Peterson [:cpeterson] from comment #14)
> Release Note Request (optional, but appreciated)
> [Why is this notable]: This bug removes a public DOM API.
> [Affects Firefox for Android]: Yes.
> [Suggested wording]: "Removed Battery Status API to reduce fingerprinting of
> users by trackers" (or the boring alternative: "Removed Battery Status API")
> [Links (documentation, blog post, etc)]:
Comment 16 User image Jorge Villalobos [:jorgev] 2016-10-31 10:18:16 PDT
(In reply to Andrea Marchesini [:baku] from comment #9)
> Jorge, we need your help here :)
> We would like to get rid of Battery API but, as you can see in Comment 8,
> there are some addons using  it.
> What do you suggest in this case?

How can these add-ons access the Battery API without using the DOM API? If it's just a matter of telling them to use an XPCOM interface instead, then that's a relatively easy fix.

If there's a real need for such an API in the long term, I agree that proposing a WebExtensions API is the way to go.
Comment 17 User image Doktor Notor 2016-11-02 05:20:10 PDT
(In reply to Chris Peterson [:cpeterson] from comment #8)
> I only found one add-on on AMO that clearly uses the battery API ("Easy
> Battery Monitor" [1]), but there are a few on DXR [2] ("Battery Logger" and
> "The Great Suspender") and the Chrome Web Store. So we should probably leave
> the battery API available to extensions for now. 
> 
> [1] https://addons.mozilla.org/en-US/firefox/addon/easy-battery-monitor/
> [2]
> https://dxr.mozilla.org/addons/
> search?q=regexp%3A%5CbgetBattery%5Cb&redirect=false

The "Easy Battery Monitor" is not compatible with anything beyond FF28, how's that even a valid attempt on an argument?

(In reply to Andrea Marchesini [:baku] from comment #9)
> We would like to get rid of Battery API but, as you can see in Comment 8,
> there are some addons using  it.
> What do you suggest in this case?

To stop supporting retarded "addons" that have no place in any browser. Why the heck would someone monitor their battery in a browser? (No, "because we can" does not count. How about an API for flashing BIOS from a browser? Might get used in "kiosk mode" as well.)

#facepalm
Comment 18 User image Chris Peterson [:cpeterson] 2016-11-02 09:53:06 PDT
WebKit implemented the battery API, though Safari didn't expose the API to web content. Yesterday the WebKit team proposed removing the API:

https://bugs.webkit.org/show_bug.cgi?id=164213

https://lists.webkit.org/pipermail/webkit-dev/2016-October/028468.html
Comment 19 User image Ritu Kothari (:ritu) 2016-11-18 13:19:44 PST
Added to Fx52 Aurora release notes
Comment 20 User image mail 2016-11-28 23:39:06 PST
Couldn't the privacy concerns be resolved similar to the Geolocation API? Ask permission, user opts in. There are real world use cases for this in responsive design. For example, an interface whose components ease off the CPU when batter gets low. Maybe they hit the API less often, lazy load, reduce animation.

https://github.com/jpdevries/mab-recommendations/blob/low-battery/proposed/low-battery.md#-responding-to-battery-levels
Comment 22 User image Chris Peterson [:cpeterson] 2017-01-17 09:56:25 PST
Thanks, Chris. LGTM.

Note You need to log in before you can comment on or make changes to this bug.