Bug 94084

Summary: [Refactoring] The debug version and the release version of toHTMLSelectElement can be merged without any penalty
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: mifenton, morrita, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 91487    
Attachments:
Description Flags
Patch none

Shinya Kawanaka
Reported 2012-08-15 01:36:17 PDT
It seems they can be merged without speed or memory penalty. We have have cleaner code.
Attachments
Patch (3.00 KB, patch)
2012-08-15 01:46 PDT, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1 2012-08-15 01:46:45 PDT
Created attachment 158525 [details] Patch
Kent Tamura
Comment 2 2012-08-15 01:56:54 PDT
Comment on attachment 158525 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158525&action=review > Source/WebCore/html/HTMLSelectElement.h:202 > +inline bool isHTMLSelectElement(const Node* node) > +{ > + return node->hasTagName(HTMLNames::selectTag); I don't think this function is meaningful.
Shinya Kawanaka
Comment 3 2012-08-15 01:59:12 PDT
(In reply to comment #2) > (From update of attachment 158525 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158525&action=review > > > Source/WebCore/html/HTMLSelectElement.h:202 > > +inline bool isHTMLSelectElement(const Node* node) > > +{ > > + return node->hasTagName(HTMLNames::selectTag); > > I don't think this function is meaningful. Ah, I just followed the recent convention (having toHTMLXXXElement and isHTMLXXXElement). Is it better to remove it?
Kent Tamura
Comment 4 2012-08-15 02:03:30 PDT
Comment on attachment 158525 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158525&action=review >>> Source/WebCore/html/HTMLSelectElement.h:202 >>> + return node->hasTagName(HTMLNames::selectTag); >> >> I don't think this function is meaningful. > > Ah, I just followed the recent convention (having toHTMLXXXElement and isHTMLXXXElement). > Is it better to remove it? It's ok to have it if you're going to use it in other places.
Shinya Kawanaka
Comment 5 2012-08-15 02:07:02 PDT
(In reply to comment #4) > (From update of attachment 158525 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158525&action=review > > >>> Source/WebCore/html/HTMLSelectElement.h:202 > >>> + return node->hasTagName(HTMLNames::selectTag); > >> > >> I don't think this function is meaningful. > > > > Ah, I just followed the recent convention (having toHTMLXXXElement and isHTMLXXXElement). > > Is it better to remove it? > > It's ok to have it if you're going to use it in other places. Yes, I'll use it.
Kent Tamura
Comment 6 2012-08-15 02:16:22 PDT
Comment on attachment 158525 [details] Patch ok
WebKit Review Bot
Comment 7 2012-08-15 02:52:12 PDT
Comment on attachment 158525 [details] Patch Clearing flags on attachment: 158525 Committed r125660: <http://trac.webkit.org/changeset/125660>
WebKit Review Bot
Comment 8 2012-08-15 02:52:15 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.