Re: [Vibration API] Confusing list indexing in spec

Yes, looks good.  Thanks.

A few more nits, if you don't mind, plus some important points at the end:

* "This document represents the consensus of the group on the scope
and features of the Vibration API. It should be noted that the group
is aware of more advanced use cases that cannot be realised using this
simpler first version. The intent is to address them in a future
revision."

This is repeated; does it need to appear twice?

* "The API is designed to tackle high-value use cases related to gaming"

I don't know what high-value means here.

* "A vibration pattern represented by a list of time entries."

Suggest putting "milliseconds" somewhere in there, to match the the
description of the other overload.

* "If the index of time is even (the earliest even entry has index 0),
vibrate the device for time milliseconds."

Suggest s/the earliest even entry/the first entry/, or at least s/the
earliest even entry/the earliest entry/.


And the material points:

* "9. An implementation may abort the algorithm at this point."

This occurs after step 3: "If pattern is 0, or an empty list, cancel
the pre-existing instance of the processing vibration patterns
algorithm, if any, and abort these steps."  But if a page can't
vibrate the device, the page also shouldn't be able to cancel other
page's vibrations.

The reason that step 9 appears where it does is because we want pages
which can't vibrate the page not to be able to observe this fact.  So
error checking on the args has to appear before we abort the
algorithm.  I suggest rejiggering the algorithm order so that we do
all the error checking upfront, then give the UA a chance to abort,
then either cancel or start a vibration.

* "Otherwise pause [HTML5] for time milliseconds."

I think this is wrong.  As I read the spec, "pause" blocks script
execution. ("While a user agent has a paused task, the corresponding
event loop must not run further tasks, and any script in the currently
running task must block.")  The vibrator does not block anything.

You may need to specify the vibrator as a separate thread.  Otherwise
maybe the vibrator "spins the event loop".

* "If the device does not provide a vibration mechanism, or it is
disabled, the user agent must silently ignore any invocations of the
vibrate() method."  Seems to me that we should still do the error
checking from the vibration algorithm, but then behave as though no
page has permission to frob the vibrator.

* "When the visibilitychange event [PAGE-VISIBILITY] is dispatched at
the Document, the user agent must run the following steps: [either
suppress or restore the vibration]".

A few problems here.  First, this only applies if the currently
running instance of the vibration algorithm was started by this
document.

Second, it's not clear in the spec what it means to "suppress" or
"restore" the vibration.

Third, my implementation in Firefox simply cancels the vibration when
the page becomes invisible.  If that matches what other
implementations do, we should consider changing the spec to match.

The reason is, this API does not deal with overlapping vibrations
gracefully.   (For example, we had a bug where, if you clicked on a
button, Firefox provided a short vibration as feedback, and that nuked
a longer vibration that the button's onclick handler was trying to
do.)  As a result, pages can't rely on providing a long vibration
pattern and running it to completion.  And since vibration patterns
need to be short, pausing and then resuming them isn't particularly
useful.

If we did want pause/resume here, we should consider changing the
current behavior where a new vibration pattern nukes another.  But I'm
not sure that's worth changing.

-Justin

On Mon, Jun 18, 2012 at 2:55 AM, Anssi Kostiainen
<anssi.kostiainen@nokia.com> wrote:
> Hi Justin,
>
> On 16.6.2012, at 0.41, ext Justin Lebar wrote:
>
>> If you want something less verbose, how about:
>>
>> "A list of numbers representing a vibration pattern.  For example, the
>> pattern [5, 10, 15] will cause the device to vibrate for 5ms, be still
>> for 10ms, and then vibrate for 15ms."
>
> Good catch. The algorithm is right (the earliest even entry has index 0), and the description was wrong.
>
> To address the issue, I changed the description of the |pattern| simply to "A vibration pattern represented by a list of time entries." and added your example to the non-normative Examples section where it fits better. The algorithm is the normative part.
>
> The Editor's Draft [ED] has been updated. A new release [TR] with this fix will be tagged later as per the W3C process. Meanwhile, I encourage you to refer to the ED for the latest.
>
> Let us know if this addresses your concern, and thanks for reporting the bug!
>
> -Anssi
>
>> [1] https://github.com/mozilla-b2g/gaia/pull/1704/files#r995646
>
> [ED] http://dev.w3.org/2009/dap/vibration/
> [TR] http://www.w3.org/TR/vibration/

Received on Monday, 18 June 2012 13:41:12 UTC