[webkit-reviews] review denied: [Bug 72010] Add new API for Vibration API spec. : [Attachment 124075] Add NavigatorVibration class and move vibration files to the Modules dir.

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Kihong Kwon
<kihong.kwon at samsung.com>'s request for review:
Bug 72010: Add new API for Vibration API spec.
https://bugs.webkit.org/show_bug.cgi?id=72010

Attachment 124075: Add NavigatorVibration class and move vibration files to the
Modules dir.
https://bugs.webkit.org/attachment.cgi?id=124075&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=124075&action=review


> LayoutTests/ChangeLog:8
> +	   * fast/dom/navigator-vibration.html: Added.

fast/dom/navigator-vibration-expected.txt is missing?

> Source/WebCore/ChangeLog:9
> +	   http://dev.w3.org/2009/dap/vibration/
> +	   Test: fast/dom/navigator-vibration.html

I would like to see more explanations about this change.

> Source/WebCore/ChangeLog:11
> +	   * CMakeLists.txt:

Here you can also add explanations for each file (if you want).

> Source/WebCore/Modules/vibration/NavigatorVibration.cpp:2
> + *  Copyright (C) 2011 Samsung Electronics

2012?

> Source/WebCore/Modules/vibration/NavigatorVibration.cpp:56
> +    ec = 0;

This may be unnecessary.

> Source/WebCore/Modules/vibration/NavigatorVibration.h:2
> + *  Copyright (C) 2011 Samsung Electronics

2012?

> Source/WebCore/Modules/vibration/NavigatorVibration.idl:2
> + *  Copyright (C) 2011 Samsung Electronics

2012?

> Source/WebCore/Modules/vibration/NavigatorVibration.idl:26
> +	   [Custom] void webkitVibrate(in Int32Array pattern)
raises(DOMException);

Please use [JSCCustom] instead of [Custom], since you support JSC only.

> Source/WebCore/Modules/vibration/Vibration.h:2
> + *  Copyright (C) 2011 Samsung Electronics

2012?

> Source/WebCore/Modules/vibration/VibrationClient.h:2
> + *  Copyright (C) 2011 Samsung Electronics

2012?