12.68 KB, patch
|Details | Diff | Splinter Review|
2.13 KB, patch
|Details | Diff | Splinter Review|
17.92 KB, patch
|Details | Diff | Splinter Review|
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)
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)
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.
(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.
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.
2 years ago
Keywords: dev-doc-needed, site-compat
2 years ago
Pushed by firstname.lastname@example.org: 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
I only found one add-on on AMO that clearly uses the battery API ("Easy Battery Monitor" ), but there are a few on DXR  ("Battery Logger" and "The Great Suspender") and the Chrome Web Store. So we should probably leave the battery API available to extensions for now.  https://addons.mozilla.org/en-US/firefox/addon/easy-battery-monitor/  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?
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: 2 years ago
status-firefox52: --- → fixed
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.
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: --- → ?
(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.
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
Added to Fx52 Aurora release notes
relnote-firefox: ? → 52+
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
Thanks, Chris. LGTM.
Maybe I am one of the few that would like to use this API. I have a full screen kiosk mode web application. The devices in question are locked down windows tablets which can only access this single webpage in full screen. They are Windows tablets so that they can easily be rolled out secured and monitored through domain policy etc. When you have a full screen kiosk mode browser you cannot see the battery life or the wifi strength. The tablets in question are attached to a large trolley which has other equipment connected to it. It is of great benefit for the user to see they have 2% battery life, instead of wheeling the device down the corridors only for them to die mid use, because someone forgot to leave them charging. The client has specific asked for the ability to display battery level and wifi strength. Sadly WIFI strength is unavailable so the closest I can get is online/offline, but the battery level is of great benefit. Right now the site has to use Chrome to gain this benefit. I felt it important to contribute as everyone is suggesting there are no real world use cases. Additionally I am not the only person with this requirement, while looking for options I cam across this post: https://stackoverflow.com/questions/39527952/chrome-extension-get-wifi-signal-strength Essentially the person has the same basic use case.
So this left BatteryManager interface exposed to the web. One can't use it, but it is there.
You need to log in before you can comment on or make changes to this bug.