Re: [battery] Battery Editors Draft update needed to prepare for LC

On Sat, 7 Jun 2014, at 8:04, cathy.chan@nokia.com wrote:
> A few comments on the changes...
> 
> In Section 5 Navigator Interface, the second step says 
> [[
> If those steps were previously successfully run in the current browsing
> context ...
> ]]
> It is not clear what "those steps" refer to. "these steps" would actually
> be more accurate. Alternatively, consider something like "If an instance
> of BatteryManager has previously been created in the current browsing
> context..."

Done.

> In Section 6 BatteryManager Interface, the following two statements are
> conflicting with regard to what the value of chargingTime should be when
> the implementation is unable to report it:
> [[
> When a BatteryManager object is created, if the implementation is unable
> to report the battery's charging state, charging time, level or remaining
> time, charging MUST be set to true, ***chargingTime to 0***, level to 1.0
> and dischargingTime to the value positive Infinity respectively.
> ]]
> [[
> 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.
> ]]
> I don't remember which one was the agreed behavior, but would note that
> the latter one works well in the multiple batteries scenario as
> described. If the former one is used, the multiple batteries description
> might need more tweaks.

So, the confusion is that when the battery is charging but not full and
the system can't report the charging time, it should report it as
Infinity. I added that precision.

> While we're looking at the attributes of BatteryManager, I would suggest
> swapping the order of the sentences within each paragraph. Begin with
> what the attribute represents, followed by how its value should be set.
> For example:
> [[
> The charging attribute represents the charging state of the system's
> battery. It 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. When the battery charging state is updated, the user agent
> MUST queue a task which sets the charging attribute's value and fires a
> simple event named chargingchange at the BatteryManager object.
> ]]
> It reads more naturally.

Hmm, I am not sure what to do here. It is purely editorial and Anssi
wrote that originally so I would prefer to let him decide how to change
the wording.

> In Examples 3 and 4:
> 	navigator.getBattery(function(battery) {
> should be 
> 	navigator.getBattery().then(function(battery) {

Good catch ;)

> Finally, clearly just a nit...
> In Example 1 & 2, clearTimeout(...) should be clearInterval(...).

Eh, that wasn't part of my changes but yes indeed :)

Thank you for the great feedback Cathy!

-- Mounir

Received on Monday, 9 June 2014 10:17:38 UTC