Bug 130345

Summary: Refine childShouldCreateRenderer for MathML elements
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, commit-queue, dbarton, mrobinson
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 124128, 130455    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch cfleizach: review+

Description Frédéric Wang (:fredw) 2014-03-17 09:50:23 PDT
Follow-up of bug 124128.
Comment 1 Frédéric Wang (:fredw) 2014-03-20 08:05:34 PDT
Created attachment 227290 [details]
Patch

This adds more restriction and more tests.
Comment 2 Frédéric Wang (:fredw) 2014-03-20 09:18:18 PDT
BTW, I've reported https://www.w3.org/Bugs/Public/show_bug.cgi?id=25104
Comment 3 chris fleizach 2014-03-20 17:30:39 PDT
Comment on attachment 227290 [details]
Patch

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

> LayoutTests/mathml/presentation/annotation-children.html:9
> +    <p><math><semantics><csymbol>Content MathML</csymbol><annotation>PA<mtext>ERROR</mtext>SS</annotation></semantics></math></p>

can you added a comment line here to indicate what should be happening

> LayoutTests/mathml/presentation/foreign-element-in-token.html:127
> +  <p>var: <math><mtext><var>mtext</var></mtext></math></p>

bad indendation

> LayoutTests/mathml/presentation/foreign-element-in-token.html:133
> +    <p>h1: <math><mi><h1>mi</h1></mi><mtext><h1>mi</h1></mtext><mn><h1>mn</h1></mn></math></p>

do we need to test all the h tags

> Source/WebCore/mathml/MathMLElement.cpp:286
> +            return htmlElement.hasTagName(HTMLNames::htmlTag) || (isFlowContent(htmlElement) && StyledElement::childShouldCreateRenderer(child));

it looks like isFlowContent is already doing the StyledElement::childShouldCreateRenderer check
Comment 4 Frédéric Wang (:fredw) 2014-03-21 07:09:23 PDT
Created attachment 227430 [details]
Patch
Comment 5 chris fleizach 2014-03-21 09:07:58 PDT
(In reply to comment #4)
> Created an attachment (id=227430) [details]
> Patch

Should this be r?
Comment 6 Frédéric Wang (:fredw) 2014-03-21 09:09:35 PDT
Comment on attachment 227430 [details]
Patch

Yes, I forgot that one.
Comment 7 Frédéric Wang (:fredw) 2014-03-21 09:20:09 PDT
Committed r166065: <http://trac.webkit.org/changeset/166065>