Bug 80237

Summary: Internally used HTMLContentElement classes should have correct wrapper.
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: DOMAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 79935    
Attachments:
Description Flags
Patch
none
Patch tkent: review+, webkit-ews: commit-queue-

Hajime Morrita
Reported 2012-03-04 21:12:07 PST
currently some HTMLContent subclasses use divTag for their tag name. It should be clearly different. Using divTag makes wrapper factory confused. It makes a wrong wrapper for if we the confusing tag name.
Attachments
Patch (17.53 KB, patch)
2012-03-04 23:36 PST, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch (17.03 KB, patch)
2012-03-05 00:00 PST, Hajime Morrita 		tkent: review+
webkit-ews: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2012-03-04 23:36:34 PST
Created attachment 130060 [details] Patch
Kent Tamura
Comment 2 2012-03-04 23:49:05 PST
Comment on attachment 130060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130060&action=review almost ok. I have some minor comments. > Source/WebCore/dom/make_names.pl:329 > my $interfaceName = $enabledTags{$tagName}{interfaceName}; > - > if ($enabledTags{$tagName}{mapToTagName}) { Removing unrelated line isn't good. > LayoutTests/fast/dom/shadow/content-element-user-agent-shadow-expected.txt:3 > +PASS parsedElementShouldBeUnknown.tagName is expectedTagName > +PASS 0 < parsedElementShouldBeUnknown.constructor.toString().indexOf('HTMLUnknownElement') is true > +PASS contentInuserAgentShadow.tagName is expectedTagName Showing 'expectedTagName' isn't good for readability. Please show WEBKITSHADOWCONTENT.
Hajime Morrita
Comment 3 2012-03-05 00:00:26 PST
Created attachment 130062 [details] Patch
Hajime Morrita
Comment 4 2012-03-05 00:01:47 PST
Kent-san, thanks for taking a look! I addressed points. > > Source/WebCore/dom/make_names.pl:329 > > my $interfaceName = $enabledTags{$tagName}{interfaceName}; > > - > > if ($enabledTags{$tagName}{mapToTagName}) { > > Removing unrelated line isn't good. True. recovered. > > > LayoutTests/fast/dom/shadow/content-element-user-agent-shadow-expected.txt:3 > > +PASS parsedElementShouldBeUnknown.tagName is expectedTagName > > +PASS 0 < parsedElementShouldBeUnknown.constructor.toString().indexOf('HTMLUnknownElement') is true > > +PASS contentInuserAgentShadow.tagName is expectedTagName > > Showing 'expectedTagName' isn't good for readability. Please show WEBKITSHADOWCONTENT. Done.
Kent Tamura
Comment 5 2012-03-05 00:03:51 PST
Comment on attachment 130062 [details] Patch ok
Early Warning System Bot
Comment 6 2012-03-05 00:21:11 PST
Comment on attachment 130062 [details] Patch Attachment 130062 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11807634
Kent Tamura
Comment 7 2012-03-05 00:23:11 PST
Comment on attachment 130062 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130062&action=review > Source/WebCore/html/shadow/HTMLContentElement.cpp:47 > + return HTMLNames::webkitShadowContentTag Need a trailing ';'
Gyuyoung Kim
Comment 8 2012-03-05 00:29:11 PST
Comment on attachment 130062 [details] Patch Attachment 130062 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11808411
Hajime Morrita
Comment 9 2012-03-05 00:38:34 PST
Committed r109721: <http://trac.webkit.org/changeset/109721>
Csaba Osztrogonác
Comment 10 2012-03-05 00:55:03 PST
(In reply to comment #9) > Committed r109721: <http://trac.webkit.org/changeset/109721> Why did you commit when Qt EWS said that you will break the build? Reopen, because you did it. Please fix the build or roll it out.
Csaba Osztrogonác
Comment 11 2012-03-05 00:55:22 PST
../../../../Source/WebCore/html/shadow/HTMLContentElement.cpp: In function 'const WebCore::QualifiedName& WebCore::contentTagName()': ../../../../Source/WebCore/html/shadow/HTMLContentElement.cpp:49: error: expected ';' before '}' token
Csaba Osztrogonác
Comment 12 2012-03-05 00:59:53 PST
I saw you fixed it by http://trac.webkit.org/changeset/109723 Next time please fix before landing. And please mention in the changelog of buildfix and in bug what you fixed. (buildfix after r.....)
Hajime Morrita
Comment 13 2012-03-05 17:00:27 PST
(In reply to comment #12) > I saw you fixed it by http://trac.webkit.org/changeset/109723 > Next time please fix before landing. And please mention in the > changelog of buildfix and in bug what you fixed. (buildfix after r.....) Aw... I'm sorry about that. I should wait green bots before landing.
Note You need to log in before you can comment on or make changes to this bug.