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

Status: RESOLVED WONTFIX

(Core :: SVG, enhancement)

Description

[Robert Longson [:longsonr]]

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.

Comment 1

[Robert Longson [:longsonr]]

Attachment: patch with reftest

Comment 2

[Daniel Holbert [: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

Comment 3

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=89fe51f4745617991629f3300e8be7f31bff1bb3

Comment 4

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f3007066d067f9cfdca133d9df2942a548f7bc9

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8450cfbdf29
comma and/or whitespace should be mandatory to separate transform values r=dholbert

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/a8450cfbdf29

Comment 7

[Kacper Michajłow [:kasper93]]

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.

Comment 8

[Robert Longson [: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.

Comment 9

[Robert Longson [:longsonr]]

Any thoughts on whether to keep this or try to get the spec changed Jonathan?

Comment 10

[Jonathan Watt [:jwatt]]

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.

Comment 11

[Robert Longson [:longsonr]]

Attachment: testcase

Comment 12

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9263d25afcd041bf301cbd96c4cb8199975c85bf

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/9263d25afcd0 (landed with wrong bug number)