Remove web content access to Battery API

VERIFIED FIXED in Firefox 52

Status

()

Core
DOM: Device Interfaces
VERIFIED FIXED
9 months ago
4 months ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

(Blocks: 2 bugs, 4 keywords)

unspecified
mozilla52
addon-compat, dev-doc-complete, privacy, site-compat
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox51 wontfix, firefox52 verified, relnote-firefox 52+)

Details

Attachments

(3 attachments)

(Assignee)

Description

9 months ago
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.
Attachment #8805443 - Flags: review?(amarchesini)
(Assignee)

Comment 1

9 months ago
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.
Attachment #8805444 - Flags: review?(amarchesini)
(Assignee)

Comment 2

9 months ago
Created attachment 8805445 [details] [diff] [review]
part-1-remove-b2g-battery-tests.patch

Part 1: Remove unused B2G tests for Battery API.
Attachment #8805445 - Flags: review?(amarchesini)
Attachment #8805445 - Flags: review?(amarchesini) → review+
Attachment #8805444 - Flags: review?(amarchesini) → review+
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?
Attachment #8805443 - Flags: review?(amarchesini) → review+
See my last comment.
Flags: needinfo?(cpeterson)
(Assignee)

Comment 5

9 months ago
(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.
Flags: needinfo?(cpeterson)
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.

Updated

9 months ago
Keywords: dev-doc-needed, site-compat

Updated

9 months ago
Blocks: 572650
Keywords: privacy

Comment 7

9 months ago
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
(Assignee)

Comment 8

9 months ago
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
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?
Flags: needinfo?(jorge)

Comment 10

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8f8f02af9d17
https://hg.mozilla.org/mozilla-central/rev/585f52662d69
https://hg.mozilla.org/mozilla-central/rev/6069dc9afe31
Status: NEW → RESOLVED
Last Resolved: 9 months ago
status-firefox52: --- → fixed
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/battery-status-api-has-been-removed/
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
> 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.
(Assignee)

Comment 14

9 months ago
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)]:
status-firefox51: --- → wontfix
relnote-firefox: --- → ?

Comment 15

9 months ago
(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)]:
(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.
Flags: needinfo?(jorge)
Keywords: addon-compat
Blocks: 1314076

Comment 17

9 months ago
(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
(Assignee)

Comment 18

9 months ago
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
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified

Comment 19

8 months ago
Added to Fx52 Aurora release notes
relnote-firefox: ? → 52+

Comment 20

8 months ago
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
I've documented this change, updating the relevant pages:

https://developer.mozilla.org/en-US/docs/Web/API/Battery_Status_API#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/API/BatteryManager#Browser_compatibility (and member child pages)
https://developer.mozilla.org/en-US/docs/Web/API/Navigator/getBattery#Browser_compatibility
https://developer.mozilla.org/en-US/Apps/Fundamentals/gather_and_modify_data/retrieving_battery_status_information#The_Battery_Status_API

and adding a note to the Fx 52 release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/52#Others

Let me know if this looks OK. Cheers!
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 22

6 months ago
Thanks, Chris. LGTM.
Comment hidden (advocacy)
You need to log in before you can comment on or make changes to this bug.