Status

()

defect
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: longsonr, Assigned: longsonr)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

11 years ago
Posted patch patch (obsolete) — Splinter Review
The only things a text frame can contain is text content:

c.f. http://www.w3.org/TR/SVG11/svgdtd.html#DTD.1.23 

<!ENTITY % SVG.TextContent.class
    "| %SVG.tspan.qname; | %SVG.tref.qname; | %SVG.textPath.qname;
     | %SVG.altGlyph.qname; %SVG.TextContent.extra.class;"
>

<!ELEMENT %SVG.text.qname; %SVG.text.content; >
Attachment #329966 - Flags: superreview?(roc)
Attachment #329966 - Flags: review?(roc)
Assignee

Updated

11 years ago
Blocks: 445582
Assignee

Comment 1

11 years ago
Posted patch updated patch (obsolete) — Splinter Review
Previous patch would have allowed <text><tspan><text>x</text></tspan></text>
Attachment #329966 - Attachment is obsolete: true
Attachment #329979 - Flags: superreview?(roc)
Attachment #329979 - Flags: review?(roc)
Attachment #329966 - Flags: superreview?(roc)
Attachment #329966 - Flags: review?(roc)
I see what the DTD says, but why should that constrain the rendering of a tree --- e.g. one that might have been constructed using DOM APIs?
Assignee

Comment 3

11 years ago
According to http://www.w3.org/TR/SVG11/implnote.html#ErrorProcessing we're not supposed to render erroneous stuff.

Anyway, we can't really render that kind of thing properly. We try to place the text child elements directly and again via their parents. Things like the effect of baseline-shift would mean that you would get a different layout depending on whether the parent or child pass happened first.

Also, if you nested deeply you would have to come back after several cups of coffee.



That section doesn't actually cover this situation, as written, but I agree that the spirit is aligned with what you're doing here.
(The spec could use clarification on that point, I suppose.)

+  if (aParentFrame && aParentFrame->GetType() == nsGkAtoms::svgTextFrame) {
+    // Text frames can only contain text contents
+    nsISVGTextContentMetrics* metrics;
+    CallQueryInterface(aParentFrame, &metrics);

How can an svgTextFrame not QI to nsISVGTextContentMetrics?
Assignee

Comment 6

11 years ago
Posted patch updated patchSplinter Review
(In reply to comment #5)

> How can an svgTextFrame not QI to nsISVGTextContentMetrics?
> 

Bad Robert, must write reftests :-(

I've given up on that bit as you can have some non text content e.g. animate and so testing for exactly what is allowed is difficult as I don't want the animation patch to stop working all of a sudden.

I'll raise a follow up as we do need to do something more sophisticated to catch invalid text contents such as rect elements without disallowing all non-text contents.
Attachment #329979 - Attachment is obsolete: true
Attachment #330549 - Flags: superreview?(roc)
Attachment #330549 - Flags: review?(roc)
Attachment #329979 - Flags: superreview?(roc)
Attachment #329979 - Flags: review?(roc)
Attachment #330549 - Flags: superreview?(roc)
Attachment #330549 - Flags: superreview+
Attachment #330549 - Flags: review?(roc)
Attachment #330549 - Flags: review+
Assignee

Comment 7

11 years ago
checked in c772ee3d2c4a

Raised bug 449215 to check children too.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.