Summary: | Use toHTMLSelectElement and dismiss isHTMLSelectElement | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kangil Han <kangil.han> | ||||||
Component: | WebCore Misc. | Assignee: | Kangil Han <kangil.han> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eflews.bot, gyuyoung.kim, kling, koivisto, rniwa | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Kangil Han
2013-07-15 23:55:29 PDT
Created attachment 206731 [details]
Patch
After having time for thought on isFooElement, I've reached a conclusion that using isFooElement would bring us more drawbacks than advantages. For example, as Ryosuke concerned, all elements aren't presented by tag names. Moreover, it will bring unnecessary header includes, it actually is refactoring though. Therefore, from now on, I will introduce or use toFooElement and dismiss isFooElement step by step. :) Comment on attachment 206731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206731&action=review > Source/WebCore/ChangeLog:9 > + Additionally, this patch dismisses isHTMLSelectElement because it doesn't cover all element and basically accompanies unnecessary header includes. I don't really understand what you mean here. Comment on attachment 206731 [details] Patch Attachment 206731 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1090104 (In reply to comment #3) > (From update of attachment 206731 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206731&action=review > > > Source/WebCore/ChangeLog:9 > > + Additionally, this patch dismisses isHTMLSelectElement because it doesn't cover all element and basically accompanies unnecessary header includes. > > I don't really understand what you mean here. How about this? :) Additionally, this patch dismisses isHTMLSelectElement because all elements aren't presented by tag names and it will bring unnecessary header includes; it actually is refactoring though. (In reply to comment #4) > (From update of attachment 206731 [details]) > Attachment 206731 [details] did not pass efl-wk2-ews (efl-wk2): > Output: http://webkit-queues.appspot.com/results/1090104 Below error is not related to this patch. :) *** Error during phase checkout of libseccomp: ########## Error running git remote update origin *** (In reply to comment #5) > (In reply to comment #3) > > (From update of attachment 206731 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=206731&action=review > > > > > Source/WebCore/ChangeLog:9 > > > + Additionally, this patch dismisses isHTMLSelectElement because it doesn't cover all element and basically accompanies unnecessary header includes. > > > > I don't really understand what you mean here. > > How about this? :) > > Additionally, this patch dismisses isHTMLSelectElement because all elements aren't presented by tag names and it will bring unnecessary header includes; it actually is refactoring though. How about /dismisses/removes/ and mention that "not all element subclasses can be checked by a combination of tag names". I don't think unnecessary header includes is applicable here. I don't think you have to say this is refactoring since the entire patch is refactoring. Created attachment 206738 [details]
Patch
(In reply to comment #7) > > How about /dismisses/removes/ and mention that "not all element subclasses can be checked by a combination of tag names". I don't think unnecessary header includes is applicable here. I don't think you have to say this is refactoring since the entire patch is refactoring. Thx Ryosuke! :) Took Ryosuke's comment into consideration. Comment on attachment 206738 [details] Patch Clearing flags on attachment: 206738 Committed r152709: <http://trac.webkit.org/changeset/152709> All reviewed patches have been landed. Closing bug. |