Bug 140063 - Modernize the HTMLElement factory
Summary: Modernize the HTMLElement factory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-04 18:42 PST by Sam Weinig
Modified: 2015-01-06 14:32 PST (History)
2 users (show)

See Also:


Attachments
Patch (108.10 KB, patch)
2015-01-04 19:47 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (109.45 KB, patch)
2015-01-04 22:27 PST, Sam Weinig
darin: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2015-01-04 18:42:26 PST
Modernize the HTMLElement factory
Comment 1 Sam Weinig 2015-01-04 19:47:39 PST
Created attachment 243955 [details]
Patch
Comment 2 WebKit Commit Bot 2015-01-04 19:48:25 PST
Attachment 243955 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:11:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 144 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Darin Adler 2015-01-04 21:14:13 PST
Comment on attachment 243955 [details]
Patch

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

> Source/WebCore/dom/make_names.pl:384
> +    my $smartPointerType = ($parameters{namespace} eq "HTML") ? "Ref" : "PassRefPtr";

I’d rather you listed the legacy namespaces that still need PassRefPtr and made HTML the normal one.
Comment 4 Sam Weinig 2015-01-04 21:50:32 PST
(In reply to comment #3)
> Comment on attachment 243955 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=243955&action=review
> 
> > Source/WebCore/dom/make_names.pl:384
> > +    my $smartPointerType = ($parameters{namespace} eq "HTML") ? "Ref" : "PassRefPtr";
> 
> I’d rather you listed the legacy namespaces that still need PassRefPtr and
> made HTML the normal one.

Good idea.
Comment 5 Sam Weinig 2015-01-04 22:27:11 PST
Created attachment 243964 [details]
Patch
Comment 6 WebKit Commit Bot 2015-01-04 22:29:27 PST
Attachment 243964 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:11:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 146 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Darin Adler 2015-01-04 22:30:55 PST
Comment on attachment 243964 [details]
Patch

Seems like later we should make the generator smart, and have it only pass the tagName if the same class is used for more than one tag. Then we can get rid of many extra create functions.
Comment 8 Darin Adler 2015-01-04 22:31:42 PST
Comment on attachment 243964 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        * Files elided *

I wouldn’t have done that (by the way), but I guess it’s OK.
Comment 9 WebKit Commit Bot 2015-01-05 11:40:36 PST
Comment on attachment 243964 [details]
Patch

Rejecting attachment 243964 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 243964, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
/html/HTMLUnknownElement.h
patching file Source/WebCore/html/HTMLVideoElement.cpp
patching file Source/WebCore/html/HTMLVideoElement.h
patching file Source/WebCore/html/HTMLWBRElement.cpp
patching file Source/WebCore/html/HTMLWBRElement.h
patching file Source/WebCore/mathml/MathMLElement.cpp
patching file Source/WebCore/mathml/MathMLElement.h

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Darin Adler']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/6306838249209856
Comment 10 Sam Weinig 2015-01-06 14:32:48 PST
Committed r177996: <http://trac.webkit.org/changeset/177996>