[webkit-reviews] review denied: [Bug 61674] <input> checkbox and radio attribute value default value incorrect : [Attachment 95537] Proposed Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 31 23:36:13 PDT 2011


Kent Tamura <tkent at chromium.org> has denied Naiem <naiem.shaik at gmail.com>'s
request for review:
Bug 61674: <input> checkbox and radio attribute value default value incorrect
https://bugs.webkit.org/show_bug.cgi?id=61674

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

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

> LayoutTests/ChangeLog:11
> +	   * fast/html/check-box-input-types-expected.txt: Added.
> +	   * fast/html/check-box-input-types.html: Added.
> +	   * fast/html/radio-input-types-expected.txt: Added.
> +	   * fast/html/radio-input-types.html: Added.

The test should be in LayoutTests/fast/forms/.
Also, the names are not good.  I propose:
  fast/forms/checkbox-default-value.html
  fast/forms/radio-default-value.html

> LayoutTests/fast/html/check-box-input-types.html:8
> +function print(message)
> +{
> +    var paragraph = document.createElement("li");
> +    paragraph.appendChild(document.createTextNode(message));
> +    document.getElementById("console").appendChild(paragraph);
> +}

You don't need to make your own function.  You can use debug() function in
fast/js/resource/js-test-pre.js.
In this case, you had better use shouldBe() function.

> LayoutTests/fast/html/check-box-input-types.html:19
> +<body onload="test()">

You don't need to run the test in onload handler.


More information about the webkit-reviews mailing list