WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
172114
Align WebKitCSSMatrix stringifier with spec for DOMMatrix
https://bugs.webkit.org/show_bug.cgi?id=172114
Summary
Align WebKitCSSMatrix stringifier with spec for DOMMatrix
Simon Pieters (:zcorpan)
Reported
2017-05-15 05:30:32 PDT
See
https://github.com/w3c/fxtf-drafts/issues/120
https://github.com/w3c/fxtf-drafts/pull/148
https://github.com/w3c/web-platform-tests/pull/5885
In WebKit the numbers are serialized always with 6 digits, per spec it should use JS's ToString. Fail WebKitCSSMatrix stringifier: identity (2d) assert_equals: expected "matrix(1, 0, 0, 1, 0, 0)" but got "matrix(1.000000, 0.000000, 0.000000, 1.000000, 0.000000, 0.000000)" NaN and Infinity values should cause an InvalidStateError exception in the stringifier.
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5168
<!DOCTYPE html> <script> var m = new WebKitCSSMatrix(); m.a = NaN; m.b = Infinity; m.c = -Infinity; w(String(m)); </script> log: matrix(nan, inf, -inf, 1.000000, 0.000000, 0.000000) Related bugs:
https://bugs.webkit.org/show_bug.cgi?id=153675
https://bugs.webkit.org/show_bug.cgi?id=110001
https://bugs.webkit.org/show_bug.cgi?id=163505
Attachments
WIP Patch
(11.39 KB, patch)
2017-05-15 12:18 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(17.93 KB, patch)
2017-05-15 12:50 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(18.05 KB, patch)
2017-05-15 14:21 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2017-05-15 09:34:29 PDT
(In reply to Simon Pieters from
comment #0
)
> See >
https://github.com/w3c/fxtf-drafts/issues/120
>
https://github.com/w3c/fxtf-drafts/pull/148
>
https://github.com/w3c/web-platform-tests/pull/5885
> > In WebKit the numbers are serialized always with 6 digits, per spec it > should use JS's ToString. > > Fail WebKitCSSMatrix stringifier: identity (2d) assert_equals: expected > "matrix(1, 0, 0, 1, 0, 0)" but got "matrix(1.000000, 0.000000, 0.000000, > 1.000000, 0.000000, 0.000000)"
Ok, it seems Firefox does this. Seems like a good thing to fix.
> > > NaN and Infinity values should cause an InvalidStateError exception in the > stringifier. > >
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5168
> > <!DOCTYPE html> > <script> > var m = new WebKitCSSMatrix(); > m.a = NaN; > m.b = Infinity; > m.c = -Infinity; > w(String(m)); > </script> > > log: matrix(nan, inf, -inf, 1.000000, 0.000000, 0.000000)
That part, I am less sure of. No other browser seems to throw and I expect compatibility risks if we started throwing.
Chris Dumez
Comment 2
2017-05-15 11:39:42 PDT
(In reply to Chris Dumez from
comment #1
)
> (In reply to Simon Pieters from
comment #0
) > > See > >
https://github.com/w3c/fxtf-drafts/issues/120
> >
https://github.com/w3c/fxtf-drafts/pull/148
> >
https://github.com/w3c/web-platform-tests/pull/5885
> > > > In WebKit the numbers are serialized always with 6 digits, per spec it > > should use JS's ToString. > > > > Fail WebKitCSSMatrix stringifier: identity (2d) assert_equals: expected > > "matrix(1, 0, 0, 1, 0, 0)" but got "matrix(1.000000, 0.000000, 0.000000, > > 1.000000, 0.000000, 0.000000)" > > Ok, it seems Firefox does this. Seems like a good thing to fix. > > > > > > > NaN and Infinity values should cause an InvalidStateError exception in the > > stringifier. > > > >
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5168
> > > > <!DOCTYPE html> > > <script> > > var m = new WebKitCSSMatrix(); > > m.a = NaN; > > m.b = Infinity; > > m.c = -Infinity; > > w(String(m)); > > </script> > > > > log: matrix(nan, inf, -inf, 1.000000, 0.000000, 0.000000) > > That part, I am less sure of. No other browser seems to throw and I expect > compatibility risks if we started throwing.
Looks like Simon is Ok with this change so I'll look into doing this part too.
Chris Dumez
Comment 3
2017-05-15 12:18:35 PDT
Created
attachment 310154
[details]
WIP Patch
Chris Dumez
Comment 4
2017-05-15 12:50:26 PDT
Created
attachment 310156
[details]
Patch
Simon Fraser (smfr)
Comment 5
2017-05-15 14:11:58 PDT
Comment on
attachment 310156
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=310156&action=review
> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:1702 > + && std::isfinite(m_matrix[3][3]);
What happened to m_matrix[3][0], m_matrix[3][1], m_matrix[3][2] ?
> LayoutTests/fast/css/matrix-stringifier-expected.txt:19 > +FAIL DOMMatrix stringifier: identity (2d) undefined is not a constructor (evaluating 'new self[constr]()') > +FAIL DOMMatrix stringifier: integer 2d undefined is not a constructor (evaluating 'new self[constr]("matrix(1, 0, 0, 1, 0, 0)")') > +FAIL DOMMatrix stringifier: integer 3d undefined is not a constructor (evaluating 'new self[constr]("matrix3d(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1)")') > +FAIL DOMMatrix stringifier: floating point 2d undefined is not a constructor (evaluating 'new self[constr]("matrix(0.1, 0.2, 0.3, 0.4, 0.5, 0.6)")') > +FAIL DOMMatrix stringifier: floating point 3d undefined is not a constructor (evaluating 'new self[constr]("matrix3d(0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7)")') > +FAIL DOMMatrix stringifier: NaN undefined is not a constructor (evaluating 'new self[constr]()') > +FAIL DOMMatrix stringifier: Infinity undefined is not a constructor (evaluating 'new self[constr]()') > +FAIL DOMMatrix stringifier: -Infinity undefined is not a constructor (evaluating 'new self[constr]()') > +FAIL ${constr}.toString should be enumerable undefined is not an object (evaluating 'self[constr].prototype') > +FAIL DOMMatrixReadOnly stringifier: identity (2d) undefined is not a constructor (evaluating 'new self[constr]()') > +FAIL DOMMatrixReadOnly stringifier: integer 2d undefined is not a constructor (evaluating 'new self[constr]("matrix(1, 0, 0, 1, 0, 0)")') > +FAIL DOMMatrixReadOnly stringifier: integer 3d undefined is not a constructor (evaluating 'new self[constr]("matrix3d(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1)")') > +FAIL DOMMatrixReadOnly stringifier: floating point 2d undefined is not a constructor (evaluating 'new self[constr]("matrix(0.1, 0.2, 0.3, 0.4, 0.5, 0.6)")') > +FAIL DOMMatrixReadOnly stringifier: floating point 3d undefined is not a constructor (evaluating 'new self[constr]("matrix3d(0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7)")') > +FAIL DOMMatrixReadOnly stringifier: NaN undefined is not a constructor (evaluating 'new self[constr]()') > +FAIL DOMMatrixReadOnly stringifier: Infinity undefined is not a constructor (evaluating 'new self[constr]()') > +FAIL DOMMatrixReadOnly stringifier: -Infinity undefined is not a constructor (evaluating 'new self[constr]()') > +FAIL ${constr}.toString should be enumerable undefined is not an object (evaluating 'self[constr].prototype')
I assume these fails are expected at this point?
Chris Dumez
Comment 6
2017-05-15 14:15:43 PDT
Comment on
attachment 310156
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=310156&action=review
>> Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp:1702 >> + && std::isfinite(m_matrix[3][3]); > > What happened to m_matrix[3][0], m_matrix[3][1], m_matrix[3][2] ?
Well, this is an excellent question :)
Chris Dumez
Comment 7
2017-05-15 14:16:53 PDT
Comment on
attachment 310156
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=310156&action=review
>> LayoutTests/fast/css/matrix-stringifier-expected.txt:19 >> +FAIL ${constr}.toString should be enumerable undefined is not an object (evaluating 'self[constr].prototype') > > I assume these fails are expected at this point?
And yes, those are expected at this point since we do not support DOMMatrix or DOMMatrixReadOnly yet. That said, the ${constr} in the message is a bug, I'll fix before landing.
Chris Dumez
Comment 8
2017-05-15 14:21:25 PDT
Created
attachment 310162
[details]
Patch
Chris Dumez
Comment 9
2017-05-15 15:08:00 PDT
Comment on
attachment 310162
[details]
Patch Clearing flags on attachment: 310162 Committed
r216881
: <
http://trac.webkit.org/changeset/216881
>
Chris Dumez
Comment 10
2017-05-15 15:08:02 PDT
All reviewed patches have been landed. Closing bug.
Simon Pieters (:zcorpan)
Comment 11
2017-05-16 14:14:20 PDT
Let me know if throwing causes problems. The reason I think it should be OK is because the current behavior will throw upon parsing anyway: var m = new WebKitCSSMatrix(); m.a = NaN; var m2 = new WebKitCSSMatrix(String(m)); // SyntaxError (DOM Exception 12): The string did not match the expected pattern.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug