Closed Bug 619630 Opened 9 years ago Closed 9 years ago

Support no comma or whitespace between points in polyline

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jwatt, Assigned: longsonr)

References

()

Details

Attachments

(2 files, 3 obsolete files)

If the "next" number in a polyline/polygon 'points' attribute is negative, then the grammar says that a space or comma is not required. (So points="270-225 300-245..." is valid.)
Duplicate of this bug: 620026
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → longsonr
Attachment #498512 - Flags: review?(jwatt)
Attached image testcase
Opera and Webkit also both pass this.
I'll leave you to decide whether to raise bugs on either Opera and Webkit or the SVG Specification.

coordinate-pairs:
    coordinate-pair
    | coordinate-pair comma-wsp coordinate-pairs
coordinate-pair:
    coordinate comma-wsp coordinate
    | coordinate negative-coordinate
so I don't see how comment 3 is valid.
You're right, it's not. In that case, can you change your test to check for that by changing it to:

<svg xmlns="http://www.w3.org/2000/svg" version="1.1">
  <title>Testing for x-y |points| attribute</title>

  <!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=619630 -->

  <rect fill="lime" height="100%" width="100%"/>
  <rect width="100" height="100" fill="red"/>

  <!-- this should render -->
  <polygon points="-10-110 -10-10 -110-10 -110-110" fill="lime"
    transform="translate(110,110)"/>

  <!-- this should not -->
  <polygon fill="red" transform="translate(110,110)"
    points="-110-10-110-110-10-110-10-10"/>
</svg>

Note the change I made to your existing path in addition to the new path. From looking at your patch I think my modifications to your existing path will cause the test to fail (because it erroneously returns an error if the Y coordinate of the last coordinate pair is negative and against the X coordinate).
Attached patch that I can do. (obsolete) — Splinter Review
Attachment #498512 - Attachment is obsolete: true
Attachment #498523 - Flags: review?(jwatt)
Attachment #498512 - Flags: review?(jwatt)
You didn't make my modifications to the existing polygon...and note what I said about your current patch failing on the modified polygon.
Attached patch reftest as jwatt wants (obsolete) — Splinter Review
Anything for a quiet life on the reftest front but your code comment isn't right as my code works just fine :-)
Attachment #498523 - Attachment is obsolete: true
Attachment #498528 - Flags: review?(jwatt)
Attachment #498523 - Flags: review?(jwatt)
(In reply to comment #6)
> Note the change I made to your existing path in addition to the new path. From
> looking at your patch I think my modifications to your existing path will cause
> the test to fail (because it erroneously returns an error if the Y coordinate
> of the last coordinate pair is negative and against the X coordinate).

It doesn't.
Attached patch fix reftest.listSplinter Review
Attachment #498528 - Attachment is obsolete: true
Attachment #498534 - Flags: review?(jwatt)
Attachment #498528 - Flags: review?(jwatt)
Comment on attachment 498534 [details] [diff] [review]
fix reftest.list

r=jwatt

Yeah, sorry, I'm gonna blame jetlag on misreading the code.

Thanks for changing both polygons in the test! I think having all the coordinate pairs test x-y is better even though your patch was correct.
Attachment #498534 - Flags: review?(jwatt) → review+
Attachment #498534 - Flags: approval2.0?
Keywords: checkin-needed
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/b2dc4b542229
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.