[webkit-reviews] review denied: [Bug 51809] [Chromium] Autofill should work with HTML5 form elements : [Attachment 79698] Fix format errors.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 20 23:06:36 PST 2011


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 79698: Fix format errors.
https://bugs.webkit.org/attachment.cgi?id=79698&action=review

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

> Source/WebCore/html/TextFieldInputType.cpp:57
> +    // All text field input like email, month, tel, and not color, time,
date, datetime, datetime-local, time, url, week can be completed.

Please remove the comment.  This helps nothing.

I meant we should have a comment on InputType::canSetSuggestedValue() (not
TextFieldInputType::canSetSuggetedValue()) so that one can understand how to
decide whether an input type should return true for canSetSuggestedValue(). 
The answer is Ilya's comment:

> I think canSetSuggestedValue() answers a question that is relevant to the
rendering engine, not the browser: Does this element have a way to display a
suggested value (one that is not accessible to JavaScript but viewable by the
user)?	The answer to this question should be independent of whether specific
browsers ever actually set suggested values.

The comment would be...
    // Should return true if the corresponding renderer for a type can display
a suggested value.

> Source/WebKit/chromium/src/WebInputElement.cpp:169
> +	   return 0;
> +
> +    return static_cast<WebInputElement*>(webElement);

We had better have
  ASSERT(inputElement->isHTMLElement());
here.

InputElement can be one of HTMLInputElement or WMLInputElement, and Chromium
doesn't support WML for now.


More information about the webkit-reviews mailing list