[webkit-reviews] review granted: [Bug 67742] Push more code from HTMLInputElement::setValue to TextFieldInputType::setValue : [Attachment 106699] Updated

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 67742: Push more code from HTMLInputElement::setValue to
TextFieldInputType::setValue
https://bugs.webkit.org/show_bug.cgi?id=67742

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=106699&action=review


> Source/WebCore/html/HTMLInputElement.cpp:1093
> +    m_suggestedValue = String(); // TextFieldInputType::setValue uses the
suggested value.

This comment does not make clear why clearing the suggested value is the
correct thing to do. Neither did the old comment.

> Source/WebCore/html/HTMLInputElement.cpp:1104
> +    // FIXME: why do we do this when !sendChangeEvent?

The word “why” should be capitalized.

> Source/WebCore/html/TextFieldInputType.cpp:98
> +    if (element()->focused())
> +	   element()->dispatchFormControlInputEvent();
> +    else
> +	   element()->dispatchFormControlChangeEvent();

I think this should use early return instead of else, and call through to the
base class dispatchChangeEventInResponseToSetValue instead of calling
dispatchFormControlChangeEvent after the early return.