Bug 15464 – SVGLengthList allows bad values

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Oliver Hunt]

SVGLengthList is too lenient on input values, eg:
<text x="1.2.3"></text>
should result in x.length == 0, but it manages to pull at least one value, 1.2, from the string.

[Rob Buis]

Created attachment 17621 [details]
First attempt

This patch fixes the problem. Note that FireFox also dejects x="1.2.3", whereas Opera accepts it. Dejecting it seems better to me, though I have trouble finding
the part of the spec detailing what syntax a decimal number has. For instance it
would be great if it could tell whether something like x="1." as an XML attribute is acceptable or not.
Cheers,

Rob.

[Eric Seidel (no email)]

1. is not valid, according to SVG 1.1:

<number> (real number value): The specification of real number values is different for property values than for XML attribute values.
CSS2 [CSS2] states that a property value which is a <number> is specified in decimal notation (i.e., a <decimal-number>), which consists of either an <integer>, or an optional sign character followed by zero or more digits followed by a dot (.) followed by one or more digits. Thus, for conformance with CSS2, any property in SVG which accepts <number> values is specified in decimal notation only.

"one or more digits" is the important phrase here.

[Rob Buis]

Hey Eric,

(In reply to comment #2)
> 1. is not valid, according to SVG 1.1:
> 
> <number> (real number value): The specification of real number values is
> different for property values than for XML attribute values.
> CSS2 [CSS2] states that a property value which is a <number> is specified in
> decimal notation (i.e., a <decimal-number>), which consists of either an
> <integer>, or an optional sign character followed by zero or more digits
> followed by a dot (.) followed by one or more digits. Thus, for conformance
> with CSS2, any property in SVG which accepts <number> values is specified in
> decimal notation only.
> 
> "one or more digits" is the important phrase here.

Thnx, I overlooked that. I just noticed FireFox and Opera handle it, but not sure if that is a bug or compatibility with existing svgs. I suppose we can wait and see whether any popular svgs or maybe svgs exported from programs (that are popular) expect this to work. I'd say we just should concentrate on the 1.2.3 problem/patch for now :)
Cheers,

Rob.

[Darin Adler]

Comment on attachment 17621 [details]
First attempt

Ideally I'd like to see the new test be a platform independent text-based test rather than one where the result is a rendered SVG.

I also am not convinced this tests all the call sites.

But despite those concerns, r=me.

[Rob Buis]

Hi Darin,

(In reply to comment #4)
> (From update of attachment 17621 [details] [edit])
> Ideally I'd like to see the new test be a platform independent text-based test
> rather than one where the result is a rendered SVG.

I am ok with redoing the testcase a bit before landing, I'll only be able to land after work anyway. Do you mean you'd rather see a text result saying PASS or FAIL, or just not landing the pixel results?

> I also am not convinced this tests all the call sites.

I think not only length lists, but also lengths can use this syntax "1.2.3". The code paths are the same for lengths and length lists regarding parsing lengths. I'll try to add a test for lengths in the testcase too.

> But despite those concerns, r=me.

Thanks. I'll wait a little bit for your reply, since there is no big rush to land this, ie. it is "just" a correctness fix.
Cheers,

Rob.

[Rob Buis]

Landed in r28563.