[webkit-reviews] review denied: [Bug 70276] Remove OptionElement (first half) : [Attachment 111347] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 18 04:04:06 PDT 2011


Kent Tamura <tkent at chromium.org> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 70276: Remove OptionElement (first half)
https://bugs.webkit.org/show_bug.cgi?id=70276

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

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


r- because of build failures.

> Source/WebCore/html/HTMLOptionElement.cpp:42
> +#include "StringBuilder.h"

Should be <wtf/text/StringBuilder.h>

> Source/WebCore/html/HTMLOptionElement.cpp:170
> +    int length = items.size();

nit: length should be size_t.

> Source/WebCore/html/HTMLOptionElement.cpp:315
> +    for (Node* n = firstChild(); n; ) {

nit: One-letter variable name is not good.

> Source/WebCore/html/HTMLSelectElement.cpp:78
> +static inline HTMLOptionElement* toOptionElement(Element* element)

This function name is confusing.  This looks like toClassName() which we use
for many classes, but there is no OptionElement class and toClassName() usually
returns no 0.
Do you have a plan to remove/rename this?


More information about the webkit-reviews mailing list