Bug 78873 – [Forms] Integrate InputType::dispatchChangeEventInResponseToSetValue into InputType::setValue

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

A virtual method InputType::dispatchChangeEventInResponseToSetValue is
* there is only one call site in HTMLInputElement::setValue
* there are two implementations in InputType and TextFiledInputType.

Merging this method into HTMLInputElement::setValue makes our code base simpler and helps to implement input/change event dispatch in Spin button (https://bugs.webkit.org/show_bug.cgi?id=75067)

Created attachment 127526 [details]
Patch 1

[Kent Tamura]

Comment on attachment 127526 [details]
Patch 1

View in context: https://bugs.webkit.org/attachment.cgi?id=127526&action=review

> Source/WebCore/ChangeLog:9
> +        This patch simplifies event dispatch in HTML input element for ease of chaning
> +        event dispatching behavior.

chaning -> changing

I'm not sure why this change will help 'changing event dispatching behavior'.  Could you explain it please?

> Source/WebCore/html/HTMLInputElement.cpp:1049
> +        if (isTextField() && focused())

Using isTextField() in HTMLInputElement.cpp is not recommended.  Will you remove it?

Created attachment 127533 [details]
Patch 1

[Kent Tamura]

Comment on attachment 127533 [details]
Patch 1

View in context: https://bugs.webkit.org/attachment.cgi?id=127533&action=review

Looks ok.

> Source/WebCore/html/TextFieldInputType.cpp:94
> +    if (valueChanged) {

nit: We prefer early exit:

if (!valueChanged)
    return;

[Kent Tamura]

Comment on attachment 127533 [details]
Patch 1

View in context: https://bugs.webkit.org/attachment.cgi?id=127533&action=review

> Source/WebCore/ChangeLog:3
> +        [Forms] Merge InputType::dispatchChangeEventInResponseToSetValue into call site

Ah, the summary is incorrect.

Created attachment 127539 [details]
Patch 3

[Kent Tamura]

Comment on attachment 127539 [details]
Patch 3

Looks ok.

Created attachment 127543 [details]
Patch 4

[Kent Tamura]

Comment on attachment 127543 [details]
Patch 4

View in context: https://bugs.webkit.org/attachment.cgi?id=127543&action=review

> Source/WebCore/ChangeLog:9
> +        and merge dispatch dispatchChangeEventInResponseToSetValue to setValue.

merge dispatch -> merge?

> Source/WebCore/html/TextFieldInputType.cpp:83
> +    // We don't ask InputType::setValue to dispatch events because of

because of -> because

Created attachment 127544 [details]
Patch 5

[Kent Tamura]

Comment on attachment 127544 [details]
Patch 5

ok

[WebKit Review Bot]

Comment on attachment 127544 [details]
Patch 5

Clearing flags on attachment: 127544

Committed r108051: <http://trac.webkit.org/changeset/108051>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.