Bug 130345 - Refine childShouldCreateRenderer for MathML elements
Summary: Refine childShouldCreateRenderer for MathML elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on: 124128 130455
Blocks:
  Show dependency treegraph
 
Reported: 2014-03-17 09:50 PDT by Frédéric Wang (:fredw)
Modified: 2014-03-21 09:20 PDT (History)
4 users (show)

See Also:


Attachments
Patch (51.89 KB, patch)
2014-03-20 08:05 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (53.35 KB, patch)
2014-03-21 07:09 PDT, Frédéric Wang (:fredw)
cfleizach: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>