Bug 80237 – Internally used HTMLContentElement classes should have correct wrapper.

• Component: DOM
• Status: RESOLVED FIXED

[Hajime Morrita]

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.

[Hajime Morrita]

Created attachment 130060 [details]
Patch

[Kent Tamura]

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]

Created attachment 130062 [details]
Patch

[Hajime Morrita]

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 on attachment 130062 [details]
Patch

ok

[Early Warning System Bot]

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 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 on attachment 130062 [details]
Patch

Attachment 130062 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11808411

[Hajime Morrita]

Committed r109721: <http://trac.webkit.org/changeset/109721>

[Csaba Osztrogonác]

(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]

../../../../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]

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]

(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.