Bug 118334 – Append overloading function for isFooElement

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Kangil Han]

is/toHTMLFormElement should use Element* for its argument

[Kangil Han]

Created attachment 205961 [details]
Patch

[Kent Tamura]

Comment on attachment 205961 [details]
Patch

The code looks worse.
Why don't you provide both of isHTMLFormElement(Node*) and isHTMLFormElement(Element*).

[Andreas Kling]

(In reply to comment #2)
> (From update of attachment 205961 [details])
> The code looks worse.
> Why don't you provide both of isHTMLFormElement(Node*) and isHTMLFormElement(Element*).

Or possibly:
template<class T> bool isHTMLFormElementNode(T*)

[Kangil Han]

Thx tkent and kling for the comments!
I will investigate to make code better. :)

[Kangil Han]

Created attachment 205977 [details]
Patch

[Kangil Han]

kling, tkent: review please? :)

[Antti Koivisto]

Comment on attachment 205977 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205977&action=review

I

> Source/WebCore/html/HTMLFormElement.h:165
> -inline bool isHTMLFormElement(Node* node)
> +template<class T>
> +inline bool isHTMLFormElement(T* t)

I think you should provide overloads instead of using templates since you only really need two cases. Error messages are clearer that way.

[Kangil Han]

(In reply to comment #7)
> I think you should provide overloads instead of using templates since you only really need two cases. Error messages are clearer that way.

Thx! I will do that. :)

[Kangil Han]

Created attachment 205994 [details]
Patch

[Antti Koivisto]

Comment on attachment 205994 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205994&action=review

> Source/WebCore/html/HTMLOptionElement.h:119
> +inline HTMLOptionElement* toHTMLOptionElement(Element* element)
> +{
> +    ASSERT_WITH_SECURITY_IMPLICATION(!element || isHTMLOptionElement(element));
> +    return static_cast<HTMLOptionElement*>(element);
> +}
> +

You don't really need the to* overloads.

[Kangil Han]

Created attachment 206000 [details]
Patch

[Kangil Han]

(In reply to comment #10)
> (From update of attachment 205994 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205994&action=review
> 
> > Source/WebCore/html/HTMLOptionElement.h:119
> > +inline HTMLOptionElement* toHTMLOptionElement(Element* element)
> > +{
> > +    ASSERT_WITH_SECURITY_IMPLICATION(!element || isHTMLOptionElement(element));
> > +    return static_cast<HTMLOptionElement*>(element);
> > +}
> > +
> 
> You don't really need the to* overloads.

Yes, right! Thx! :)
Took anttik's comment into consideration.

[Antti Koivisto]

Comment on attachment 206000 [details]
Patch

Actually maybe you need it. I guess ASSERT_WITH_SECURITY_IMPLICATION might get compiled in in release mode too?

[Antti Koivisto]

Comment on attachment 206000 [details]
Patch

Never mind, apparently it is just a fuzzer hint.

[WebKit Commit Bot]

Comment on attachment 206000 [details]
Patch

Clearing flags on attachment: 206000

Committed r152353: <http://trac.webkit.org/changeset/152353>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.