Bug 94084 - [Refactoring] The debug version and the release version of toHTMLSelectElement can be merged without any penalty
Summary: [Refactoring] The debug version and the release version of toHTMLSelectElemen...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Shinya Kawanaka
URL:
Keywords:
Depends on:
Blocks: 91487
  Show dependency tree / graph
 
Reported: 2012-08-15 01:36 PDT by Shinya Kawanaka
Modified: 2012-08-15 02:52 PDT (History)
4 users (show)

See Also:

Attachments
Patch (3.00 KB, patch)
2012-08-15 01:46 PDT, Shinya Kawanaka 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Shinya Kawanaka 2012-08-15 01:36:17 PDT 
It seems they can be merged without speed or memory penalty.
We have have cleaner code.
Comment 1 Shinya Kawanaka 2012-08-15 01:46:45 PDT 
Created attachment 158525 [details]
Patch
Comment 2 Kent Tamura 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.
Comment 3 Shinya Kawanaka 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?
Comment 4 Kent Tamura 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.
Comment 5 Shinya Kawanaka 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.
Comment 6 Kent Tamura 2012-08-15 02:16:22 PDT 
Comment on attachment 158525 [details]
Patch

ok
Comment 7 WebKit Review Bot 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>
Comment 8 WebKit Review Bot 2012-08-15 02:52:15 PDT 
All reviewed patches have been landed.  Closing bug.