[webkit-reviews] review denied: [Bug 62840] Optimize HTMLInputElement::updateCheckedRadioButtons : [Attachment 100267] fix the checkbox issue & improve updateCheckedRadioButtons()

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied zeng huiqing
<huiqing.zeng at intel.com>'s request for review:
Bug 62840: Optimize HTMLInputElement::updateCheckedRadioButtons
https://bugs.webkit.org/show_bug.cgi?id=62840

Attachment 100267: fix the checkbox issue & improve updateCheckedRadioButtons()
https://bugs.webkit.org/attachment.cgi?id=100267&action=review

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


> Source/WebCore/ChangeLog:3
> +	   Fix the bug Optimize HTMLInputElement::updateCheckedRadioButtons

nit:
The summary looks a little curious.
"Optimize HTMLInputELement::updateCheckedRadioButton()" is enough.

> Source/WebCore/ChangeLog:14
> +	   (WebCore::Document::getFormElements):
> +	   * html/HTMLInputElement.cpp:
> +	   (WebCore::HTMLInputElement::updateCheckedRadioButtons):
> +	   (WebCore::HTMLInputElement::setChecked):

nit:
We had better explanations about what is changed in each of functions.

> Source/WebCore/html/HTMLInputElement.cpp:890
> +    if (type() != (InputTypeNames::checkbox())) {

if (isRadioButton())
would be natural.

> Source/WebCore/html/HTMLInputElement.cpp:892
> +	   setNeedsValidityCheck();

setNeedsValidaityCheck() should be called in a case of checkbox.
EWS failure of fast/forms/interactive-validation-required-checkbox.html was
caused by this change.