Bug 264752

Summary:	Negative SVGTransform scale values should be correctly stringified
Product:	WebKit	Reporter:	Ahmad Saleem <ahmad.saleem792>
Component:	SVG	Assignee:	Nobody <webkit-unassigned>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	karlcow, sabouhallawa, webkit-bug-importer, zimmermann
Priority:	P2	Keywords:	BrowserCompat, InRadar
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified
See Also:	https://bugs.webkit.org/show_bug.cgi?id=267401

Description Ahmad Saleem 2023-11-13 10:03:53 PST

Hi Team,

While going through Blink's commit, I came across another failing test case:

Test Case: https://jsfiddle.net/7nqg2L01/show

^ Firefox Nightly 121:

FAIL g.getAttribute('transform') should be scale(-2 -4). Was scale(-2, -4).

^ Chrome Canary 121:

PASS g.getAttribute('transform') is "scale(-2 -4)"

^ Safari Technology Preview 182 (Don't show negative):

FAIL g.getAttribute('transform') should be scale(-2 -4). Was scale(2 4).

___

Blink Commit: https://chromium.googlesource.com/chromium/blink/+/8a7252a2602bd8067c83d48c8fd525ef322708c2

Just wanted to raise, so we can fix it.

Thanks!

Comment 1 Radar WebKit Bug Importer 2023-11-20 10:04:14 PST

<rdar://problem/118656892>

Comment 2 Karl Dubost 2023-11-20 18:08:04 PST

There are potentially two different issues 
* negative to positive number
* serialization with/without comma
as the 3 browsers return something different. 



         IN             OUT
Safari:  scale(-2, -4)  scale(2 4).   No comma, positive numbers
Firefox: scale(-2, -4)  scale(-2, -4) Comma, negative numbers
Chrome:  scale(-2, -4)  scale(-2 -4)  No comma, negative numbers

I have the feeling that Firefox is right here for the comma.
https://drafts.csswg.org/css-transforms/#funcdef-transform-scale
I didn't find a reference why it should be serialize differently, aka without the comma. 


But WebKit definitely needs to return negative numbers.

Comment 3 Karl Dubost 2023-11-20 18:09:47 PST

The test:

var svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
var g = document.createElementNS('http://www.w3.org/2000/svg', 'g');
var transform = svg.createSVGTransform();
transform.setScale(-2, -4);
g.transform.baseVal.appendItem(transform)
g.getAttribute('transform')

Comment 4 Ahmad Saleem 2024-01-06 17:51:27 PST

We need to update this:

https://searchfox.org/wubkat/rev/c22ca98693565713efde37e8311190dd00e165ef/Source/WebCore/svg/SVGTransformValue.h#267

void appendScale(StringBuilder& builder) const
    {
        appendFixedPrecisionNumbers(builder, m_matrix->value().xScale(), m_matrix->value().yScale());
    }

TO:

void appendScale(StringBuilder& builder) const
    {
        appendFixedPrecisionNumbers(builder, m_matrix->a(), m_matrix->d());
    }

__

Compiles and make us pass but match 'Chrome'.

As for ',' issue highlighted in Comment 02. I think it might be 'Parser' issue? I am happy to fix this and then have separate bug to discuss ',' issue.

Comment 5 Ahmad Saleem 2024-01-06 18:03:16 PST

Draft PR - https://github.com/WebKit/WebKit/pull/22475

To see if it fails anything else on EWS. :-)

Comment 6 EWS 2024-01-10 21:17:24 PST

Committed 272885@main (2efdea0ba354): <https://commits.webkit.org/272885@main>

Reviewed commits have been landed. Closing PR #22475 and removing active labels.