Summary: | Append overloading function for isFooElement | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kangil Han <kangil.han> | ||||||||||
Component: | WebCore Misc. | Assignee: | Kangil Han <kangil.han> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, kling, koivisto, tkent | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Kangil Han
2013-07-02 19:04:45 PDT
Created attachment 205961 [details]
Patch
Comment on attachment 205961 [details]
Patch
The code looks worse.
Why don't you provide both of isHTMLFormElement(Node*) and isHTMLFormElement(Element*).
(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*) Thx tkent and kling for the comments! I will investigate to make code better. :) Created attachment 205977 [details]
Patch
kling, tkent: review please? :) 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. (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. :) Created attachment 205994 [details]
Patch
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. Created attachment 206000 [details]
Patch
(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. 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?
Comment on attachment 206000 [details]
Patch
Never mind, apparently it is just a fuzzer hint.
Comment on attachment 206000 [details] Patch Clearing flags on attachment: 206000 Committed r152353: <http://trac.webkit.org/changeset/152353> All reviewed patches have been landed. Closing bug. |