[webkit-reviews] review granted: [Bug 70220] Remove OptionGroupElement : [Attachment 111225] Patch 2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 17 10:16:09 PDT 2011


Darin Adler <darin at apple.com> has granted Kent Tamura <tkent at chromium.org>'s
request for review:
Bug 70220: Remove OptionGroupElement
https://bugs.webkit.org/show_bug.cgi?id=70220

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=111225&action=review


> Source/WebCore/dom/OptionElement.cpp:112
> +    if (parentElement && isHTMLOptGroupElement(parentElement))

I’m not sure we should have a special function for this. Just doing
hasTagName(optgroupTag) directly seems fine to me.

> Source/WebCore/html/HTMLOptGroupElement.cpp:147
> +    return node && node->isHTMLElement() && node->hasTagName(optgroupTag);

There is no need for the isHTMLElement check in this function. The hasTagName
function already checks the namespace and will return false for
non-HTMLElement.

> Source/WebCore/html/HTMLOptGroupElement.h:64
> +bool isHTMLOptGroupElement(const Node*);

As I said above, I am not sure we need a helper function. It’s the same thing
as calling hasTagName(optgroupTag).


More information about the webkit-reviews mailing list