[webkit-reviews] review denied: [Bug 71128] Select multiple options with mouse drag in Select element. : [Attachment 112892] Proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 2 01:27:40 PDT 2011


Kent Tamura <tkent at chromium.org> has denied Rakesh <rakesh.kn at motorola.com>'s
request for review:
Bug 71128: Select multiple options with mouse drag in Select element.
https://bugs.webkit.org/show_bug.cgi?id=71128

Attachment 112892: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=112892&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=112892&action=review


> Source/WebCore/ChangeLog:8
> +	   Selection in select element with an mouse drag.

Please add rationale to ChangeLog.

> LayoutTests/fast/forms/select-multiple-elements-with-mouse-drag.html:5
> +<link rel="stylesheet" href="../js/resources/js-test-style.css">

This line is not needed. js-test-pre.js automatically loads it.

> LayoutTests/fast/forms/select-multiple-elements-with-mouse-drag.html:11
> +    if(window.eventSender) {

Though we don't have an official code style for JavaScript, please follow the
C++ style as possible.
Need a space after 'if'

> LayoutTests/fast/forms/select-multiple-elements-with-mouse-drag.html:19
> +	   eventSender.mouseMoveTo(x,y);

Need a space after ','

> LayoutTests/fast/forms/select-multiple-elements-with-mouse-drag.html:21
> +	   eventSender.mouseMoveTo(x,y + (optionHeight * 3));

Need a space after ','

> LayoutTests/fast/forms/select-multiple-elements-with-mouse-drag.html:33
> +    for(var i=0; i < 4; i++) {

Need a space
 after 'for'
 before and after '='

> LayoutTests/fast/forms/select-multiple-elements-with-mouse-drag.html:34
> +	   shouldBeTrue("document.getElementById(\"selectId\").options["+ i +
"].selected");

Need a space before the first '+'


More information about the webkit-reviews mailing list