Bug 78873

Summary:

[Forms] Integrate InputType::dispatchChangeEventInResponseToSetValue into InputType::setValue

Product:

WebKit

Reporter:

yosin

Component:

Forms

Assignee:

yosin

Status:

RESOLVED FIXED

Severity:

Normal

CC:

antaryami.pandia, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

75067

Attachments:

Description	Flags
Patch 1	none
Patch 1	none
Patch 3	none
Patch 4	none
Patch 5	none

yosin

Reported 2012-02-16 21:28:12 PST

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)

Attachments
Patch 1 (5.31 KB, patch) 2012-02-16 22:52 PST, yosin	no flags	Details Formatted Diff Diff
Patch 1 (6.30 KB, patch) 2012-02-16 23:45 PST, yosin	no flags	Details Formatted Diff Diff
Patch 3 (7.36 KB, patch) 2012-02-17 00:02 PST, yosin	no flags	Details Formatted Diff Diff
Patch 4 (7.95 KB, patch) 2012-02-17 00:27 PST, yosin	no flags	Details Formatted Diff Diff
Patch 5 (7.94 KB, patch) 2012-02-17 00:33 PST, yosin	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

yosin

Comment 1 2012-02-16 22:52:00 PST

Created attachment 127526 [details] Patch 1

Kent Tamura

Comment 2 2012-02-16 23:21:42 PST

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?

yosin

Comment 3 2012-02-16 23:45:52 PST

Created attachment 127533 [details] Patch 1

Kent Tamura

Comment 4 2012-02-16 23:51:10 PST

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 5 2012-02-16 23:51:41 PST

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.

yosin

Comment 6 2012-02-17 00:02:40 PST

Created attachment 127539 [details] Patch 3

Kent Tamura

Comment 7 2012-02-17 00:05:00 PST

Comment on attachment 127539 [details] Patch 3 Looks ok.

yosin

Comment 8 2012-02-17 00:27:11 PST

Created attachment 127543 [details] Patch 4

Kent Tamura

Comment 9 2012-02-17 00:30:15 PST

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

yosin

Comment 10 2012-02-17 00:33:12 PST

Created attachment 127544 [details] Patch 5

Kent Tamura

Comment 11 2012-02-17 00:35:11 PST

Comment on attachment 127544 [details] Patch 5 ok

WebKit Review Bot

Comment 12 2012-02-17 01:22:57 PST

Comment on attachment 127544 [details] Patch 5 Clearing flags on attachment: 127544 Committed r108051: <http://trac.webkit.org/changeset/108051>

WebKit Review Bot

Comment 13 2012-02-17 01:23:02 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.