- From: Justin Lebar <jlebar@mozilla.com>
- Date: Wed, 9 Nov 2011 14:39:23 -0500
- To: Anssi Kostiainen <anssi.kostiainen@nokia.com>, Jonas Sicking <jonas@sicking.cc>
- Cc: public-device-apis@w3.org
>> Algorithm step 5: "If pattern is 0, an empty list," **null, or undefined** > > There was earlier discussion that including null and undefined may not make sense so they were > dropped: > > http://lists.w3.org/Archives/Public/public-device-status/2011Oct/0048.html > > I added null and undefined back for now, this should be a minor issue anyway. I don't think it's quite as bad as the following looks: > vibrate(undefined) > vibrate() > vibrate(null) > vibrate(0) > vibrate([]) The last two obviously have to be there. But then any one of the first three implies the other two. In particular, I think |vibrate()| is less clunky than vibrate(0) or vibrate([]). Jonas, do you still think we should get rid of the first three above? I guess I don't feel strongly either way. I have a few more comments; mostly nits. > "If pattern is 0, an empty list, null or undefined" Nit: "null, or undefined", unless the W3C has a rule against the Oxford comma [1]. > If pattern is a list, proceed to the next step, otherwise run the following substeps: Nit: This is a comma splice, the second comma should be a semicolon or a period. > For each entry in pattern, run the following substeps: If entry exceeds an implementation dependent limit, the user agent may throw a NotSupportedError exception and abort these steps. Nit: s/implementation dependent/implementation-dependent Nit: You could write this without a loop, as: "If any element of pattern exceeds an implementation-dependent limit..." > If the length of pattern is greater than 1 and divides by 2 leaving no remainder Use "even" here, please. > Otherwise pause [HTML5] for a period of time milliseconds. Nit: "pause for |time| milliseconds." (You "Pause for a period of time," but you "pause for 42 milliseconds.") > If the hidden attribute [PAGE-VISIBILITY] is set to true, the user agent must pause [HTML5] the pre-existing instance of the processing vibration patterns algorithm, if any. > If the hidden attribute [PAGE-VISIBILITY] is set to false, the user agent must resume the pre-existing instance of the processing vibration patterns algorithm, if any. This does not match up with what we're currently doing in Firefox. We currently just cancel the vibration and don't resume when we go back to the page. This behavior may be more sane, although it's harder to do properly, since it requires sync'ing a browser timer with the internal vibration timer. When I tell Android to start vibrating, I don't know if it starts running that pattern immediately. If you're going to spec it this way, you should specify what it means to pause the algorithm while a vibration is playing -- presumably you mean that we should cancel the current vibration and resume however many ms are left when the page becomes visible again. > 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. Maybe we should have a non-normative section saying that the UA may allow the user to disable vibration globally, per-origin, or however else it wants. [1] http://thegloss.com/beauty/why-the-oxford-comma-is-something-you-should-care-about-392/ On Wed, Nov 9, 2011 at 7:58 AM, Anssi Kostiainen <anssi.kostiainen@nokia.com> wrote: > Hi Justin, > > On 4.11.2011, at 0.35, ext Justin Lebar wrote: > >> At the meeting today I said I'd send out some thoughts on the vibrator >> spec after reading it. Here they are. >> >> Section 3.1 says no exceptions for vibrate, but it may throw NotSupportedError. > > Defined NotSupportedError exception in 3.1. > >> I don't think you need the |already started| flag. It guards >> cancellations of the algorithm, but you can just unconditionally >> "cancel the pre-existing instance of the processing vibration >> algorithm, if any". > > I agree we can simplify the algorithm by removing the flag. The flag is now gone. > >> Need to indicate that if the device doesn't have a vibrator, or the >> user has disabled the vibrator (globally, or for this page), we >> silently ignore the vibration request. > > Added the following prose: > > [[ > > 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. > > ]] > >> Algorithm step 2: If a page is hidden and calls vibrate(...), why do >> we cancel other instances of the algorithm? > > I removed this, and defined additional steps for visibilitychange event to handle background tab pause and resume. > >> Algorithm step 5: "If pattern is 0 or an empty list, cancel the >> pre-existing attempt, if any," What does it mean to cancel the >> pre-existing attempt? Is that the same as canceling the pre-existing >> instance of the processing vibration patterns algorithm? If this is >> annoying to type, let's declare a more concise synonym. > > Lets call it "the pre-existing instance of the processing vibration patterns algorithm" to be explicit. Or if you have a better name in mind, just let me know. Now the spec refers to this term consistently. > >> Algorithm step 5: "If pattern is 0, an empty list," **null, or undefined** > > There was earlier discussion that including null and undefined may not make sense so they were dropped: > > http://lists.w3.org/Archives/Public/public-device-status/2011Oct/0048.html > > I added null and undefined back for now, this should be a minor issue anyway. > >> Algorithm step 7: Throw if any entry is not a non-negative number. > > I made the vibrate() arguments of type 'unsigned long' and 'unsigned long[]'. This should relegate throwing exceptions for negative numbers to Web IDL. > >> Algorithm steps 8, 10: Can we use "even" instead of "divides by 2 >> leaving no remainder"? If you don't want to rely on the fact that 0 >> is even, you can always test for oddness. > > Changed to simpler form as per your suggestion. > > I checked the algorithm for processing vibration patterns against your manual test case, but it would be great if you could double check it against your implementation and let me know if there are any further issues: > > http://dev.w3.org/2009/dap/vibration/ > > Thanks for your extremely helpful feedback! > > -Anssi >
Received on Wednesday, 9 November 2011 19:40:23 UTC