Re: [Vibration API] Confusing list indexing in spec

Hi Justin, All,

On 18.6.2012, at 16.40, ext Justin Lebar wrote:

> Yes, looks good.  Thanks.
> 
> A few more nits, if you don't mind, plus some important points at the end:

Thanks! Actually, people who report spec bugs, nits, and provide constructive feedback are the most awesome!

I'll address most of the issues below, and the rest in a reply to Robin re reordering the algorithm itself.

[...]

> * "The API is designed to tackle high-value use cases related to gaming"
> 
> I don't know what high-value means here.

Removed the weasel word "high-value".

> * "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.

Now the spec says:

[[

A vibration pattern represented by a list of time entries, in milliseconds.

]]

> * "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/.

I did s/the earliest even entry/the first 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.

I reordered the algorithm as Robin proposed in his reply.

> * "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".

I changed the spec to say:

[[

Otherwise spin the event loop for /time/ milliseconds.

]]

> * "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.

This should be addressed by the reordered algorithm.

> * "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.

I made this more explicit by adding "in a browsing context".

[I also added a Terminology section.]

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

See below.

> 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.

I changed the spec to match your implementation:

[[

When the visibilitychange event [PAGE-VISIBILITY] is dispatched at the Document in a browsing context, the user agent must cancel the vibration.

]]

Please look at the updated Editor's Draft for any bugs:

  http://dev.w3.org/2009/dap/vibration/

(the diff to previous: http://dev.w3.org/cvsweb/2009/dap/vibration/Overview.html.diff?r1=1.24;r2=1.25;f=h)

I'll do another update next week based on feedback received.

Thanks! You've been added to the Acknowledgements section :)

-Anssi

Received on Wednesday, 20 June 2012 11:01:55 UTC