Closed
Bug 717722
Opened 13 years ago
Closed 9 years ago
implement (WebKit)CSSMatrix()
Categories
(Core :: DOM: CSS Object Model, defect)
Core
DOM: CSS Object Model
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: gal, Assigned: wchen)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-needed)
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
baku
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
baku
:
review+
|
Details | Diff | Splinter Review |
It seems popular with web authors.
What compatibility gains do we get from implementing it? Can you point at some popular sites? Is this a priority on mobile devices or generally?
Dean Jackson posted a proposed draft spec:
http://lists.w3.org/Archives/Public/public-fx/2012JanMar/0007.html
Seems like a reasonable feature.
Is CSSMatrix the same thing as Matrix4x4?
If Matrix4x4 is intended to be The Matrix To Rule Them All, then yeah that sounds like a good thing.
Same functionality --- more, actually --- but it's not quite the same ... WebkitCSSMatrix::multiply returns the new matrix, whereas Matrix4x4::multiply modifies the matrix in-place. But Matrix4x4 offers multipliedBy to return a new matrix.
Should we ask Dean to make Matrix4x4 more of a drop-in replacement for WebkitCSSMatrix?
Something like |typedef Matrix4x4 WebkitCSSMatrix;| seems like a desirable goal to me.
Similarly, I would also love to have a reasonable solution to bug 683051, a matrix type that makes sense in canvas APIs. Having a Matrix3x3 where |typedef Matrix3x3 SVGMatrix|;, or even |typedef Matrix4x4 SVGMatrix|, would solve that problem well.
Comment 7•11 years ago
|
||
Several authors have written Javascript polyfills for WebKitCSSMatrix so I'm guessing this could be a useful/desired feature in FF. Maybe some of the JS classes could work as a starting point for implementing it in Firefox?
I posted a demo on codepen (linked below) of a JS class I wrote. It's received a decent amount of attention, and makes me think that web authors might want this.
codepen demo: http://cdpn.io/mvqab (only works in Chrome/Safari, since it's comparing WebKitCSSMatrix)
github repo: https://github.com/jonbrennecke/CSSMatrix
Comment 8•10 years ago
|
||
Comment 9•10 years ago
|
||
Yes, we ship DOMMatrix already. This bug is about WebKitCSSMatrix.
Updated•10 years ago
|
Keywords: dev-doc-needed
Comment 10•10 years ago
|
||
This bug predates DOMMatrix, are you considering also having WebKitCSSMatrix? I've added a use counter in Blink to find out if we can remove it once DOMMatrix ships.
Comment 11•10 years ago
|
||
> are you considering also having WebKitCSSMatrix?
If we discover that it's necessary for compat, yes. I'm hoping it won't be.
Comment 12•10 years ago
|
||
This sounds like a good candidate for WebCompat telemetry.
I added it to the list https://wiki.mozilla.org/Compatibility/Telemetry#CSS
Comment 13•10 years ago
|
||
The Blink use counter is https://www.chromestatus.com/metrics/feature/timeline/popularity/630
It's still a long way from the stable channel but already at ~0.04%, which is very discouraging.
If it's impossible to remove, then making it an alias of DOMMatrix seems like the most promising path forward.
Comment 14•10 years ago
|
||
I see usage is up to 0.1% on the chromestatus board. :/
Doing some testing of a shim for Bug 907674 (which has like 7 dupes on webcompat.com), mapping WebKitCSSMatrix to DOMMatrix is pretty straightforward (and seems to do the job). However, it gets messy when you look at how it's actually used on ehow (and probably everywhere else on the web):
> // var e = document.defaultView.getComputedStyle(foo, null),
> // t = new WebKitCSSMatrix(e.webkitTransform);
Seems like a very common pattern: <https://github.com/search?l=javascript&q=new+WebKitCSSMatrix%28window.getComputedStyle%28el%2C+null%29.webkitTransform%29&type=Code&utf8=%E2%9C%93>
(Defining getters/setters for webkitTransform and webkitTransition on CSSStyleDeclaration.prototype does eventually get the site to work).
Comment 15•10 years ago
|
||
We are going to support -webkit prefix in CSS only on some whitelisted sites. If this support improves some high-volume real-world sites, it would deserve to consider.
Comment 16•10 years ago
|
||
Masatoshi, see Bug 1107378 about some of the prefixes being rewritten for certain chinese Web sites.
Flags: needinfo?(VYV03354)
Updated•10 years ago
|
Comment 18•10 years ago
|
||
(In reply to Mike Taylor [:miketaylr] from comment #14)
> I see usage is up to 0.1% on the chromestatus board. :/
And it hasn't even reached the stable channel yet... this isn't looking good at all.
Comment 19•10 years ago
|
||
Digging for info on MDN.
https://developer.mozilla.org/en-US/docs/Web/API/CSSMatrix
Comment 20•9 years ago
|
||
smo.suumo.jp is broken, see https://webcompat.com/issues/1145#issuecomment-105702104
> if (!window.WebKitCSSMatrix) return;
Updated•9 years ago
|
Comment 21•9 years ago
|
||
It seems like https://compat.spec.whatwg.org/#webkitcssmatrix-interface is defining things, right?
Comment 22•9 years ago
|
||
Yeah, that's where the spec for this will live (still WIP, obviously. ^_^).
Updated•9 years ago
|
Blocks: compat-spec
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → wchen
Comment 23•9 years ago
|
||
Note here one open issue about possibly making WebKitCSSMatrix an alias of DOMMatrix: https://github.com/whatwg/compat/issues/19
Assignee | ||
Comment 24•9 years ago
|
||
This patch trivially shadows the WebKitCSSMatrix methods to call into DOMMatrix methods. The only non-minor change is the addition of the WebKitCSSMatrix version of |rotate| because we don't have an equivalent in DOMMatrix.
Deviations from the spec (these are things we should fix in the spec):
- In the step for |rotate|, we first set rotX to 0 if undefined otherwise we may end up trying to rotate by undefined.
- |rotate| post multiplies the axis rotation matrices in the order of Z, Y then X since it looks like that is what WebKit is doing.
- WebKitCSSMatrix doesn't keep track of whether a matrix is 2D while DOMMatrix does so the |rotate| algorithm should set the is2D flag appropriately. In the patch I set it to false if we try and rotate about the X or Y axis by a non-zero angle.
(In reply to Mike Taylor [:miketaylr] from comment #23)
> Note here one open issue about possibly making WebKitCSSMatrix an alias of
> DOMMatrix: https://github.com/whatwg/compat/issues/19
If we are willing to break DOMMatrix in order to merge it with WebKitCSSMatrix then we may want to consider going one step further and just renaming DOMMatrix to WebKitCSSMatrix after the merge since doesn't seem like there is anything to gain from having an alias.
Attachment #8706741 -
Flags: review?(amarchesini)
Comment 25•9 years ago
|
||
>+ double rotX = aRotX.WasPassed() ? aRotX.Value() : 0;
You should just give it a default value in the IDL, no?
Similar for scale(), where scaleX and scaleZ can just have default values in IDL. scaleY will have to continue as-is.
Assignee | ||
Comment 26•9 years ago
|
||
Uploaded wrong version of the patch. I've also gone ahead and made bz's suggested IDL changes.
We should also update the IDL in the spec:
- For the scale method, scaleX and scaleZ should have default value of 1
- For the rotate method, rotX should have a default value of 0
- Remove language dealing with the arguments above being undefined.
Attachment #8706741 -
Attachment is obsolete: true
Attachment #8706741 -
Flags: review?(amarchesini)
Attachment #8706748 -
Flags: review?(amarchesini)
Comment 27•9 years ago
|
||
Comment on attachment 8706748 [details] [diff] [review]
Implement WebKitCSSMatrix
Review of attachment 8706748 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/base/DOMMatrix.h
@@ +146,1 @@
> {
no virtual DTOR ?
::: dom/base/WebKitCSSMatrix.cpp
@@ +11,5 @@
> +
> +namespace mozilla {
> +namespace dom {
> +
> +static const double radPerDegree = 2.0 * M_PI / 360.0;
sRadPerDegree
@@ +21,5 @@
> + return obj.forget();
> +}
> +
> +already_AddRefed<WebKitCSSMatrix>
> +WebKitCSSMatrix::Constructor(const GlobalObject& aGlobal, const nsAString& aTransformList, ErrorResult& aRv)
80chars max.
@@ +44,5 @@
> +
> +WebKitCSSMatrix*
> +WebKitCSSMatrix::SetMatrixValue(const nsAString& aTransformList, ErrorResult& aRv)
> +{
> + DOMMatrix::SetMatrixValue(aTransformList, aRv);
if (NS_WARN_IF(aRv.Failed()) {
return nullptr;
}
::: dom/base/WebKitCSSMatrix.h
@@ +25,5 @@
> +
> + static already_AddRefed<WebKitCSSMatrix>
> + Constructor(const GlobalObject& aGlobal, ErrorResult& aRv);
> + static already_AddRefed<WebKitCSSMatrix>
> + Constructor(const GlobalObject& aGlobal, const nsAString& aTransformList, ErrorResult& aRv);
80chars max here and in the rest of the file.
Attachment #8706748 -
Flags: review?(amarchesini) → review+
Assignee | ||
Comment 28•9 years ago
|
||
This patch puts this feature behind the same pref that we use for other webkit prefixed features to address dbaron's comment to the intent to ship: https://groups.google.com/d/msg/mozilla.dev.platform/DmOgvnSuq74/98uYCAElAgAJ
Attachment #8708240 -
Flags: review?(amarchesini)
Comment 29•9 years ago
|
||
Release Note Request (optional, but appreciated)
[Why is this notable]: WebKitCSSMatrix has been available for years in WebKit based browsers and has seen widespread usage on the web. This feature is
currently available in Chrome, Safari and Edge. Mike Taylor has been working on standardizing the feature and it is being implemented in Gecko to improve web compatibility.
[Suggested wording]: Added support for WebKitCSSMatrix to improve the mobile Web experience
[Links (documentation, blog post, etc)]:
*Link to standard*:
https://compat.spec.whatwg.org/#webkitcssmatrix-interface
https://compat.spec.whatwg.org/#dom-window-orientation
*Estimated or target release*: Firefox 46
relnote-firefox:
--- → ?
Comment 30•9 years ago
|
||
Filed https://github.com/whatwg/compat/issues/30 for spec updates (will be working on this today, but leaving a link for posterity).
Updated•9 years ago
|
Attachment #8708240 -
Flags: review?(amarchesini) → review+
Comment 31•9 years ago
|
||
Comment 32•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment 33•9 years ago
|
||
Note: this will stay in aurora until everything is ready (it's behind the same layout.css.prefixes.webkit pref as the other webkit CSS stuff: https://bugzilla.mozilla.org/show_bug.cgi?id=1238827)
Removing this from the beta 46 release notes. This is back in the relnote:? pool but over in bug 1213126 now.
relnote-firefox:
46+ → ---
Added to Fx47 release notes.
relnote-firefox:
--- → 47+
Comment 36•9 years ago
|
||
We're still not shipping this pref in 47, at least according to the code currently on beta.
Could you remove it from the release notes?
Flags: needinfo?(rkothari)
Comment 37•9 years ago
|
||
(Specifically: this feature is guarded by the pref "layout.css.prefixes.webkit", which is not being enabled for Beta/Release builds until Firefox 48 at the earliest. That enabling will happen in bug 1259345, once we're confident that it's not going to cause trouble once 48 moves to beta.)
Updated•9 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
(In reply to David Baron [:dbaron] ⌚️UTC-7 from comment #36)
> We're still not shipping this pref in 47, at least according to the code
> currently on beta.
>
> Could you remove it from the release notes?
Got it. Done.
relnote-firefox:
47+ → ---
Flags: needinfo?(rkothari)
You need to log in
before you can comment on or make changes to this bug.
Description
•