Closed Bug 1449075 Opened 2 years ago Closed Last year

Comma and/or whitespace should be mandatory to separate transform values

Categories

(Core :: SVG, enhancement)

enhancement
Not set

Tracking

()

RESOLVED WONTFIX
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: longsonr, Assigned: longsonr)

Details

Attachments

(2 files)

transform="scale(.2.2)" is invalid as is transform="translate(-100-100)" but we use the path parser to parse them which is rather laissez faire in that regard.
Assignee: nobody → longsonr
Attachment #8962581 - Flags: review?(dholbert)
Comment on attachment 8962581 [details] [diff] [review]
patch with reftest

Review of attachment 8962581 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

::: dom/svg/SVGTransformListParser.cpp
@@ +122,4 @@
>      if (*aParsedCount == aMaxCount) {
>        return false;
>      }
> +    RangedPtr<const char16_t> iter(mIter); 

fix trailing whitespace
Attachment #8962581 - Flags: review?(dholbert) → review+
Pushed by longsonr@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8450cfbdf29
comma and/or whitespace should be mandatory to separate transform values r=dholbert
https://hg.mozilla.org/mozilla-central/rev/a8450cfbdf29
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Hi,

I see the reasoning behind this patch, but I'm little bit worried about breaking real websites. I noticed that I no longer see some icons on https://naekranie.pl/ (search and login icon on the top bar). After some digging I found this change. And indeed they have invalid SVG files, but at the same time those icons work in every major browser, tested with: Edge, Chrome, IE, Opera (latest stable versions as of today). So my question is do we want to introduce this requirement despite some possible breakage? Given that everyone supports those invalid SVG currently, I suspect there might be quite a few of them in the wild.
Flags: needinfo?(longsonr)
Feel free to mail w3c SVG (https://lists.w3.org/Archives/Public/www-svg/2018Apr/) to see whether they will change the SVG specification.
Flags: needinfo?(longsonr)
Any thoughts on whether to keep this or try to get the spec changed Jonathan?
Flags: needinfo?(jwatt)
Keywords: dev-doc-needed
I suppose we should back this out if all other browsers support it, despite it being a bit yuck. :/ Maybe put a comment in the code about supporting this weird behavior for compat reasons too.
Flags: needinfo?(jwatt) → needinfo?(longsonr)
Attached image testcase
Attachment #8971789 - Attachment mime type: text/plain → image/svg+xml
Flags: needinfo?(longsonr)
Status: REOPENED → RESOLVED
Closed: 2 years agoLast year
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.