[webkit-reviews] review denied: [Bug 69701] Remove redundant code of HTMLSelectElement. : [Attachment 110270] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Oct 8 10:38:06 PDT 2011


Ryosuke Niwa <rniwa at webkit.org> has denied Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 69701: Remove redundant code of HTMLSelectElement.
https://bugs.webkit.org/show_bug.cgi?id=69701

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110270&action=review


r- because of two behavioral changes. Otherwise the patch looks good.

> Source/WebCore/html/HTMLSelectElement.cpp:724
> +void HTMLSelectElement::setSelectedIndexInternal(int optionIndex, bool
deselect, bool fireOnChangeNow, bool userDrivenChange)

I think we can just keep the function name and overload the other one.

> Source/WebCore/html/HTMLSelectElement.cpp:836
> +    setNeedsValidityCheck();

Why are we adding this function call here? It'll be best to separate this
behavioral change.

> Source/WebCore/html/HTMLSelectElement.cpp:1301
> +    HTMLFormControlElementWithState::defaultEventHandler(event);

Again, we should really separate this behavioral change.


More information about the webkit-reviews mailing list