w3c / fxtf-drafts

Mirror of https://hg.fxtf.org/drafts
https://drafts.fxtf.org/
Other
71 stars 50 forks source link

[geometry] DOMMatrixReadOnly string init depends on backwards compatibility syntax #149

Closed fserb closed 7 years ago

fserb commented 7 years ago

Spec: https://drafts.fxtf.org/geometry/#dom-dommatrixreadonly-dommatrixreadonly

The If init is a DOMString step 3 references Syntax of the SVG ‘transform’ attribute of css transform spec. That syntax is there, according to CSS transform spec:

To provide backwards compatibility, the syntax of the [SVG] transform presentation attribute differs from the syntax of the [CSS] transform style property as shown in the example above. [...] Authors are advised to follow the rules of CSS Values and Units Module.

It doesn't make much sense to target a new spec with a syntax that is there only for backwards compatibility. I propose we change this 3rd item to depend on the main CSS transform style syntax defined on CSS Values and Units.

Was there an underlying issue to reference DOMMatrix init with the backwards compatible SVG transform?

AmeliaBR commented 7 years ago

It's probably there because of the history of DOMMatrix, originally starting as part of SVG. Many methods are the way they are (e.g., using degrees instead of radians) for backwards compatibility with SVG.

But since this particular method (constructor from a string) did not exist in SVG 1.1, I agree that there is no reason to use the SVG 1.1 transform syntax.

However, see issue #122, about not wanting to have DOMMatrix methods depend on the availability of a full CSS parser.

fserb commented 7 years ago

Yeah, it seems to be the case that they settled in exposing it for Window and not for Worker (which is what is implemented on Chrome). But even there, it's using CSS parser. Using the SVG transform parser would be even worst, considering all the arguments on #122.

zcorpan commented 7 years ago

It seems the Chromium and WebKit (testing WebKitCSSParser) do not support omitting commas, but Gecko does.

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5157

Since WebKitCSSMatrix has the most legacy use on the Web, and WebKit does not support the "lax" syntax, there shouldn't be a strong web compat constraint to switching to the stricter CSS 'transform' syntax.

zcorpan commented 7 years ago

https://lists.w3.org/Archives/Public/public-fx/2014AprJun/0121.html https://lists.w3.org/Archives/Public/public-fx/2014AprJun/0132.html has rationale for using SVG syntax. In particular @dirkschulze @cabanier claimed these are common in SVG, and DOMMatrix is supposed to replace SVGMatrix.

zcorpan commented 7 years ago

However, SVGMatrix does not have a constructor, so there shouldn't be any SVG legacy to be compatible with. https://www.w3.org/TR/SVG11/coords.html#InterfaceSVGMatrix

Browsers also don't support a constructor for SVGMatrix. http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5158

zcorpan commented 7 years ago

So after reading the old thread, the argument was not to be compatible with scripts using SVGMatrix, but being able to pass in transform attribute from SVG to new scripts using DOMMatrix.

I think arbitrary SVG content can still be supported in new scripts if we only support CSS syntax by using getComputedStyle. http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5159 shows that there is currently lack of interop between Chromium and Gecko -- Chromium throws for (3), Gecko throws for (1) and (4).

Also it appears that neither browser supports transform="rotate(5deg)" in SVG, so that parser would need to be changed in implementations if DOMMatrix is to use the SVG transform parser, to not break web content that relies on new WebKitCSSMatrix('rotate(5deg)') to work.

In conclusion, I think DOMMatrix right now seems better off using CSS 'transform' rules.

zcorpan commented 7 years ago

Also cc @tabatkins who participated in the above-cited email thread.

zcorpan commented 7 years ago

Spec PR: https://github.com/w3c/fxtf-drafts/pull/153

zcorpan commented 7 years ago

WPT PR: https://github.com/w3c/web-platform-tests/pull/5902

fserb commented 7 years ago

That's great. Thanks for the PR. :)

zcorpan commented 7 years ago

@dbaron any objection here from Gecko?

dbaron commented 7 years ago

The change is making the DOMMatrix constructor take CSS syntax (with units) rather than SVG (unitless)? It sounds fine to me assuming you've got a handle on any compatibility issues.

zcorpan commented 7 years ago

Yes.

The SVG syntax is defined in terms of CSS syntax, but also allowing unitless (and some other differences), but it's not implemented that way in browsers I believe.

Since WebKit and Chromium (and likely also EdgeHTML) use the CSS parser for WebKitCSSMatrix and that is what is most widely used, it seems to be the more web-compatible approach.

Thanks!

dirkschulze commented 7 years ago

@zcorpan Does it use the strict mode for parsing? I could see users doing something like

new DOMMatrix(svgElement.getAttribute('transform'));

transform is a presentation attribute in SVG. So it can be set using an element attribute. SVG transform attributes must be compatible to the SVG syntax.

In WebKit we implemented transform as a presentation attributes and it does get parsed by the CSS parser. The attribute value does not get parsed in strict mode allowing unit-less values for instance.

I am fine with using the CSS parser but we should definitely not use the strict mode!

zcorpan commented 7 years ago

Is non-strict with CSS parser something that is implemented in all browser engines?

dirkschulze commented 7 years ago

@zcorpan It is the legacy mode IIRC. CC @tabatkins

zcorpan commented 7 years ago

Here's a test

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5240

From what I understand of the css-transforms spec, the first and the last should work, and the middle one shouldn't (space before ( is invalid).

In Gecko/WebKit/Chromium/EdgeHTML 15, the first two are rotated and the last one isn't, which is per SVG 1.1 rules I believe.

This suggests that the legacy SVG-compatible CSS syntax rules in the css-transforms spec is not actually used for the SVG transform attribute in these browser engines.

Given WebKitCSSMatrix, we have a different legacy to worry about, namely this pattern:

var matrix = new WebKitCSSMatrix(element.style.transform);

Using the SVG codepath where rotate(5deg) is not supported would break.

trusktr commented 7 years ago

So after reading the old thread, the argument was not to be compatible with scripts using SVGMatrix, but being able to pass in transform attribute from SVG to new scripts using DOMMatrix.

You guys should all just forget about having this type of API. As @AmeliaBR pointed out above, this type of API doesn't belong in DOMMatrix. Intead, specific APIs (like SVG or CSS) should have separate APIs for accepting and giving DOMMatrix instance.

For example, it could be like (just an example, may be completely off):

getComputedStyle(someElement).transform.toDOMMatrix()
getComputedStyle(someElement).transform = new DOMMatrix([...]) // sets matrix() or matrix3d()

but you get the idea, those other places can house the specific APIs for dealing with DOMMatrices as needed.

There is CSS TypedOM coming soon hopefully, which can give us such APIs.

SVG can have it's own API (if it doesn't already, I'm don't know), and it can even work with the unitless no-comma format for generating DOMMatrix instances, or for getting instances, or whatever.

The following should simply not be allowed:

new DOMMatrix(svgElement.getAttribute('transform'));

I am fine with using the CSS parser but we should definitely not use the strict mode!

Please no, let's have separation of the concerns. DOMMatrix is a mathematical number library, and if we allow people to create these things from strings, we're defeating the purpose. We need to speed up the web, and we should encourage numerical usage (as far as passing the into / getting from CSS or SVG APIs) if we're going to make new imperative APIs, otherwise the DOM already exists, and that can take in all the string-based values like it already does.

zcorpan commented 7 years ago

Indeed in https://github.com/w3c/fxtf-drafts/issues/122 the argument was made that the string argument should be dropped completely, but we can't because web compat requires it for WebKitCSSMatrix. But then WebKitCSSMatrix uses the strict CSS parser in implementations.

SVG2 already has APIs to directly return a DOMMatrix for an element, e.g.:

rect.transform.baseVal.consolidate().matrix

I think browsers still return SVGMatrix for those, but it's easy to convert them to a DOMMatrix:

DOMMatrix.fromMatrix(rect.transform.baseVal.consolidate().matrix)

@dirkschulze, ok to close this issue?

zcorpan commented 7 years ago

Closing. Happy to reconsider this if majority of browser engines no longer match the spec.

trusktr commented 7 years ago

Why do we want backwards compatibility? DOMMatrix doesn't have to have this API. CSSMatrix can wrap a DOMMatrix and provide the API while at the same not inflating DOMMatrix with unnecessary API, for backcompat.

trusktr commented 7 years ago

I meant, Why do we want backwards compatibility to live specifically in DOMMatrix?

zcorpan commented 7 years ago

We want compat with web content that uses WebKitCSSMatrix, and having that be an alias rather than a subclass makes for a simpler/cleaner implementation. But that is slightly off topic for this issue.

trusktr commented 7 years ago

It doesn't have to be a subclass, it can contain a DOMMatrix.

So, someone wants to make a WebGL app using web standards, and will never use CSS for the WebGL app, but by using DOMMatrix they may bring in heavy stuff? If I use DOMMatrix just for numerical values in WebGL, will the initialization of the CSS parser be avoided? Just making sure I don't have something heavier than I need in my own WebGL stuff.

zcorpan commented 7 years ago

@trusktr please continue this discussion in https://github.com/w3c/fxtf-drafts/issues/122 or file a new issue. This issue is about SVG syntax.