Update Battery Status API to match latest Editors draft: navigator.getBattery(), etc

RESOLVED INCOMPLETE
(Needinfo from 2 people)

Status

()

defect
RESOLVED INCOMPLETE
5 years ago
2 years ago

People

(Reporter: cpeterson, Assigned: syamgk01, Mentored, NeedInfo)

Tracking

({meta})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox34 affected, b2g-v2.1 affected)

Details

(Whiteboard: [good first bug][lang=c++], )

Reporter

Description

5 years ago
The W3C published a revised draft spec (03 July 2014) of the Battery Status API:

https://dvcs.w3.org/hg/dap/raw-file/tip/battery/Overview.html

The following changes have been made since the last published version:

 * Expose BatteryManager via getBattery() returning a Promise instead of
   a synchronous accessor (navigator.battery).
 * Clarify defaults when a BatteryManager object is created.
 * Specify the behavior when a host device has more than one battery.

Google just landed Chrome support for the revised Battery Status API:

https://code.google.com/p/chromium/issues/detail?id=122593
I'd definitely be in favor of updating our implementation to match the spec. Try checking with Jst or with Ken Chang to see if anyone on their teams could do it.
Reporter

Comment 2

5 years ago
JST or Ken: who would be an appropriate developer to update our Battery Status API to match the revised W3C spec (and Chrome's implementation)? This might be a good first/second bug.
Flags: needinfo?(kchang)
Flags: needinfo?(jst)
Version: 30 Branch → Trunk
Kan-Ru, I wonder if you could handle this bug.
Flags: needinfo?(kchang) → needinfo?(kchen)
Clearing needinfo? as Ken seems to be covering this one. If not, happy to help dig for resources here.
Flags: needinfo?(jst)
(In reply to Ken Chang[:ken] from comment #3)
> Kan-Ru, I wonder if you could handle this bug.

I think this is a good first/second bug. If someone wants to work on this I could mentor. Otherwise this has a lower priority and I'll keep a eye on it.
Mentor: kchen
Flags: needinfo?(kchen)
Whiteboard: [good first bug][lang=c++]
Hi, I'm interested by this bug, could you assign me for it?
Assignee: nobody → baylac.felix
Since I am a newbie at firefox hacking, I am likely to do something wrong… 
So I will explain what I am planning to do for these three issues. I also need some directions :)

1- Clarify defaults when a BatteryManager object is created
	Ressources:
		https://dvcs.w3.org/hg/dap/raw-file/tip/battery/Overview.html#the-batterymanager-interface
		https://hg.mozilla.org/mozilla-central/file/f41a267983c1/dom/battery/BatteryManager.cpp
		https://hg.mozilla.org/mozilla-central/file/f41a267983c1/dom/battery/BatteryManager.h
		https://hg.mozilla.org/mozilla-central/file/f41a267983c1/dom/battery/Constants.h
	
	Everything looks good for me but maybe I missed something…


2- Expose BatteryManager via getBattery() returning a Promise instead of a synchronous accessor
   (navigator.battery).
	Ressources:
		https://hg.mozilla.org/mozilla-central/file/f41a267983c1/dom/battery/BatteryManager.cpp
		https://hg.mozilla.org/mozilla-central/file/f41a267983c1/dom/battery/BatteryManager.h
		https://hg.mozilla.org/mozilla-central/file/f41a267983c1/dom/webidl/Navigator.webidl
		https://dvcs.w3.org/hg/dap/raw-file/tip/battery/Overview.html#the-navigator-interface

	What I plan to do:
		1- Specify the getBattery() method into the dom/webidl/Navigator.webidl interface.
		2- Implement the getBatttery() method into the BatteryManager class using 
                   the w3c three steps.
		3- Create the corresponding mochitest.

	Questions:
		- Do you have any examples about how to use the mozilla::dom::Promise class? (I can't grep one)

3- Specify the behavior when a host device has more than one battery.
	Ressources:
		https://dvcs.w3.org/hg/dap/raw-file/tip/battery/Overview.html#multiple-batteries
		https://hg.mozilla.org/mozilla-central/file/f41a267983c1/hal/sandbox/PHal.ipdl
	I have no idea how to do that… hal::BatteryInformation doesn't seem to handle that situation,
        so I guess that we have to implement this into the hal layer. 
        I will need some directions for that.


What do you think about this Kan-Ru?
Reporter

Comment 8

5 years ago
Félix, your plan looks good! Because each of these tasks is self-contained, I recommend creating separate bug reports for each task and tackling each task serially. #1 looks pretty straightforward and a good place to start. Those separate bugs can block this "meta bug", which we can use to track all tasks related to matching the W3C's 03 July 2014 spec changes.

If you have questions about building Firefox, drop by the #introduction IRC channel on irc.mozilla.org. For code questions, try #webapi or #developers.
Félix, yeah, your plan looks good! About dom::Promise I think you could take a look at how dom/src/notification/Notification.cpp uses it (Bug 899574), dom/datastore (a bit complicated) and dom/telephony.

For multiple batteries we might have to modify BatteryInformation to include that information and do the calculation in upper layer. I think we could defer this since there isn't a hal that supports this and it's a rare hardware configuration..
No longer blocks: 1050746
Depends on: 1050746
Depends on: 1050749
Depends on: 1050752
Ok, I've created the "sub" bugs. Feel free to modify anything if I made something wrong. 
I can't assign myself to these bugs. Can somebody assign me to those?

Comment 11

5 years ago
(In reply to Félix Baylac-Jacqué (:NinjaTrappeur) from comment #10)
> Ok, I've created the "sub" bugs. Feel free to modify anything if I made
> something wrong. 
> I can't assign myself to these bugs. Can somebody assign me to those?

I granted you the editbugs permission which will let you do this yourself in the future.  :-)
I am not working any more on this bug. Feel free to work on it.
Assignee: baylac.felix → nobody
Summary: Update Battery Status API to match W3C's 03 July 2014 draft spec: navigator.getBattery(), etc → Update Battery Status API to match latest Editors draft: navigator.getBattery(), etc
(Please avoid referencing things on the W3C's TR space - that space is for IP lawyers and government officials. Only use the latest editors drafts.)

Comment 14

5 years ago
I'd like to give it a go :)
Duplicate of this bug: 884925
There's a difference in implementation between Chrome and Gecko around when battery is full and power cable is plugged.

Firefox: battery.charging == false and battery.chargingTime == Infinity
Chrome: battery.charging == true and battery.chargingTime == 0

I would say that Chrome's behavior is more reasonable for an API. In all cases I can think of, battery.charging should indicate if the power cable is plugged, and chargingTime is used to present how much charging is needed (in this case, 0).

With Firefox behavior I have no way to identify when power is plugged when the battery is full.

Should I file a separate bug for that or does it make sense to fix it as part of this bug?
Actually, the same test on FxOS gave me the same result as Chrome (charginTime=0, level=1, charging=yes). I'm wondering if it is platform specific or intermittent. In either case, we should unify it :)
Reporter

Comment 18

4 years ago
(In reply to Zibi Braniecki [:gandalf] from comment #16)
> There's a difference in implementation between Chrome and Gecko around when
> battery is full and power cable is plugged.
> 
> Firefox: battery.charging == false and battery.chargingTime == Infinity
> Chrome: battery.charging == true and battery.chargingTime == 0

These results could very well be platform-specific, but Firefox should probably try to normalize any platform-specific quirks. Chrome's and FxOS' results seem to better match the 2014-12-09 spec:

The charging attribute MUST be set to false if the battery is discharging, and set to true, if the battery is charging, the implementation is unable to report the state, or there is no battery attached to the system, or otherwise.

The chargingTime attribute MUST be set to 0, if the battery is full or there is no battery attached to the system, and to the value positive Infinity if the battery is discharging, the implementation is unable to report the remaining charging time, or otherwise.
Assignee

Comment 19

4 years ago
Hey Can i start working on this?
(In reply to SgK from comment #19)
> Hey Can i start working on this?

Sure, thanks!
Assignee: nobody → syamgk01

Comment 21

4 years ago
I am newbie .
can somebody assign this bug to me? How to start working on this bug .I seek of little guidance.
Have you checked out and compiled Firefox?
Flags: needinfo?(punitvara)

Comment 23

4 years ago
Hello, I'm interested in working on this if no one is currently taking this on, and I already have the Firefox codebase built and compiled. I'm not sure how far this bug has been working on, and should I start looking at the sub bugs based on the comments I have read here?
Kan-Ru, see comment 23?
Flags: needinfo?(kchen)
(In reply to Lynn Tran from comment #23)
> Hello, I'm interested in working on this if no one is currently taking this
> on, and I already have the Firefox codebase built and compiled. I'm not sure
> how far this bug has been working on, and should I start looking at the sub
> bugs based on the comments I have read here?

Welcome, Lynn. So you have the Firefox built, great! I think you could start by looking at the sub bugs which already have some initial patch that could be improved upon. If you have any question please don't hesitate to leave a message here or send me a mail.
Flags: needinfo?(kchen)

Comment 26

3 years ago
Firefox Nightly fails the test "setting iframe's src makes its Navigator object vary thus getting another battery promise".

Test report: https://w3c.github.io/test-results/battery-status/20160513.html
Test case: http://www.w3c-test.org/battery-status/battery-promise.html
Editor's Draft: https://w3c.github.io/battery/

Fixing this remaining bug would mean Firefox passes the test suite and the spec could advance to its final stage.
That test is wrong.  Setting src in that situation should reuse the window from the initial about:blank, per spec, so has the same Navigator object.
Flags: needinfo?(anssi.kostiainen)

Comment 29

2 years ago
Battery API is no longer supported.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.