Closed Bug 445687 Opened 14 years ago Closed 14 years ago

Should not be able to nest text

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: longsonr, Assigned: longsonr)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached 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)
Blocks: 445582
Attached 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?
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?
Attached 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+
checked in c772ee3d2c4a

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