[webkit-reviews] review denied: [Bug 51809] [Chromium] Autofill should work with HTML5 form elements : [Attachment 79555] Add toWebInputElement().

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Naoki Takano
<takano.naoki at gmail.com>'s request for review:
Bug 51809: [Chromium] Autofill should work with HTML5 form elements
https://bugs.webkit.org/show_bug.cgi?id=51809

Attachment 79555: Add toWebInputElement().
https://bugs.webkit.org/attachment.cgi?id=79555&action=review

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

> Source/WebCore/html/InputType.cpp:469
>  {
> -    return false;
> +    return isTextField();

Please add a comment about what type should return true.

InputType::canSetSuggestedValue() should return false, and
TextFieldInputType::canSetSuggestedValue() should return true. isTextField() is
a virtual function and it costs a little bit.

> WebKit/chromium/ChangeLog:13
> +	   [Chromium] Autofill should work with HTML5 form elements
> +	   https://bugs.webkit.org/show_bug.cgi?id=51809
> +	   http://crbug.com/65654
> +
> +	   No new tests, because this fix is for Chromium project and hard to
test only in WebKit project.
> +
> +	   * public/WebInputElement.h: Added toWebInputElement() declarations.
> +	   * src/WebInputElement.cpp:
> +	   (WebKit::WebInputElement::toWebInputElement): Implemented two
convert functions to cast (const or non-const) WebNode* to (const or non-const)
WebInputElement*.

Please explain why we need toWebInputElement().

> WebKit/chromium/src/WebInputElement.cpp:167
> +    PassRefPtr<Node> node = *webNode;

We don't need to use PassRefPtr<>.