[webkit-reviews] review denied: [Bug 63709] Disabled input type=search still allows clicking [x] to clear contents : [Attachment 99295] Patch

Thu Jun 30 07:16:26 PDT 2011

Kent Tamura <tkent at chromium.org> has denied Kentaro Hara <haraken at google.com>'s
request for review:
Bug 63709: Disabled input type=search still allows clicking [x] to clear
contents
https://bugs.webkit.org/show_bug.cgi?id=63709

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

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

BTW, Does NSSearchField show the [X] button when it is disabled/readonly?

> LayoutTests/fast/forms/search-cancel-button-mouseup.html:2
>  <html>

This file is for 'cancel-button-mouseup'. So merging a test case of this bug is
not suitable.
We have disabled-search-input.html.  It's a better place to add the test case.

> LayoutTests/fast/forms/search-cancel-button-mouseup.html:13
> +<input id="search2" type="search" value="value2"><br />
> +<input id="search3" type="search" value="value3" disabled="disabled"><br />
> +<input id="search4" type="search" value="value4" disabled="disabled"><br />
> +<input id="search5" type="search" value="value5" disabled="disabled"><br />

You fixes 'readonly' too.  So you should make test cases for 'readonly' too.

> LayoutTests/fast/forms/search-cancel-button-mouseup.html:22
> +    var input = document.getElementById("search0");

nit: You can use getElementsByTagName('input') and index-access. Also,
resources/common.js has $().

> LayoutTests/fast/forms/search-cancel-button-mouseup.html:43
> +    eventSender.mouseMoveTo(middleX, y);
> +    eventSender.mouseDown();
> +    eventSender.mouseMoveTo(buttonX, y);
> +    eventSender.mouseUp();

You had better have click(x, y) function.

> Source/WebCore/ChangeLog:5
> +	   Disallow clicking an [X] button in 'search' input forms when
'disabled' attribute is set.

Please mention 'readonly' too.