[Webkit-unassigned] [Bug 72010] Add new API for Vibration API spec.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Jan 29 20:22:58 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=72010
--- Comment #35 from Kihong Kwon <kihong.kwon at samsung.com> 2012-01-29 20:22:56 PST ---
(From update of attachment 124419)
View in context: https://bugs.webkit.org/attachment.cgi?id=124419&action=review
Thank you for your comment again...:)
>> LayoutTests/fast/dom/navigator-vibration.html:13
>> +
>
> You can now remove these.
OK. I missed it.
>> LayoutTests/fast/dom/navigator-vibration.html:15
>> +shouldBe("navigator.webkitVibrate(0); navigator.webkitVibrate([]);", "undefined");
>
> Sorry for my previous bad comments! These tests can be written better as follows, right?
>
> shouldBe("navigator.webkitVibrate(0)", "undefined");
> shouldBe("navigator.webkitVibrate(1000)", "undefined");
> shouldBe("navigator.webkitVibrate([])", "undefined");
> shouldBe("navigator.webkitVibrate([1000, 300, 500])", "undefined");
>
> In addition, it is a good idea to add the following test cases:
>
> navigator.webkitVibrate() // This should be equivalent to navigator.webkitVibrate(0)
> navigator.webkitVibrate(-1)
> navigator.webkitVibrate(4294967295)
> navigator.webkitVibrate(4294967296)
> navigator.webkitVibrate(1.23)
OK, but I can't check '4294967295' with JSC because it doesn't support unsigned long properly.
Instead, I will check '2147483647'. Can I make it this way?
>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:71
>> +JSValue JSNavigator::webkitVibrate(ExecState* exec)
>
> Would you please make a new file "JSNavigatorVibrationCustom.cpp" and write this method in the file? (c.f. JSDOMWindowWebSocketCustom.cpp, JSDOMWindowWebAudioCustom.cpp)
No problem.
>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:80
>> + pattern.append(value.asInt32());
>
> This might be "value.asUInt32()" (, although asUInt32() just calls asInt32() internally.)
OK.
>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:81
>> + else if (JSObject* object = toJSSequence(exec, value, length)) {
>
> I guess we need to fallback to NOT_SUPPORTED_ERR if toJSSequence() returns an exception. So the logic would be
>
> if (value.isUInt32()) {
> ...;
> } else {
> JSObject* object = toJSSequence(...);
> if (exec->hadException()) {
> exec->clearException();
> setDOMException(exec, NOT_SUPPORTED_ERR);
> return jsUndefined();
> }
> if (!length)
> ...;
> ...;
> }
Yes. You are better.
>>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:90
>>> + if (value.isUndefinedOrNull() || !value.isNumber()) {
>>
>> This might be "if (value.isUndefinedOrNull() || !value.isUInt32())".
>
> Maybe this should be just "if (!value.isUInt32())"?
You are right.
>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:91
>> + setDOMException(exec, INVALID_ACCESS_ERR);
>
> This should be NOT_SUPPORTED_ERR, according to the spec: http://dev.w3.org/2009/dap/vibration/#vibration-interface
Yes.
>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:96
>> + pattern.append(value.asInt32());
>
> value.asUInt32()
Yes.
>> Source/WebCore/bindings/js/JSNavigatorCustom.cpp:101
>> + setDOMException(exec, INVALID_ACCESS_ERR);
>
> NOT_SUPPORTED_ERR
Yes.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list