Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 719054 - matrix() and matrix3d() with length units should be parse errors
: matrix() and matrix3d() with length units should be parse errors
: dev-doc-complete
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: x86 Linux
: -- minor (vote)
: mozilla16
Assigned To: Aryeh Gregor (:ayg) (working until November 1)
: Jet Villegas (:jet)
Depends on: 774475 774169
Blocks: 745523
  Show dependency treegraph
Reported: 2012-01-18 08:19 PST by Aryeh Gregor (:ayg) (working until November 1)
Modified: 2014-12-28 05:32 PST (History)
3 users (show)
ayg: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (18.92 KB, patch)
2012-02-13 08:31 PST, Aryeh Gregor (:ayg) (working until November 1)
no flags Details | Diff | Splinter Review
Patch v2, rebased (18.97 KB, patch)
2012-06-26 07:40 PDT, Aryeh Gregor (:ayg) (working until November 1)
dbaron: review+
Details | Diff | Splinter Review

Description Aryeh Gregor (:ayg) (working until November 1) 2012-01-18 08:19:23 PST
The CSS Transform specs give the syntax of matrix() and matrix3d() as taking <number>s only:

matrix(<number>, <number>, <number>, <number>, <number>, <number>)

matrix3d(<number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>, <number>)

Firefox 12.0a1 (2012-01-17) incorrectly accepts <length> as well as <number> for the translation parts of these matrices, like matrix(1, 0, 0, 1, 10px, 10px).  Simple test-case:

data:text/html,<!doctype html>
<div style=-moz-transform:matrix(1,0,0,1,10px,10px)></div>

This should output the empty string.  It doesn't in Firefox, but does in all other browsers I tested, per spec.  This causes Firefox to fail some of the CSS Transforms tests I'm submitting to the W3C.

(Relatedly, the computed value of transform should not include "px", but I'm not testing that yet because it's not totally clear in the spec:
Comment 1 David Baron :dbaron: ⌚️UTC+9 (busy until November 7) 2012-01-18 09:48:08 PST
We should change this in the same release that we unprefix.  (Though I think maybe we should try to convince other browsers to change.)
Comment 2 Matt Woodrow (:mattwoodrow) 2012-01-18 19:48:32 PST
This was definitely an intentional choice to provide flexibility for authors, consistency with the translate() functions, and consistency with our existing matrix() implementation.

I'd prefer to see this allowed in the spec than remove it from our code.
Comment 3 Aryeh Gregor (:ayg) (working until November 1) 2012-01-19 08:16:49 PST
If other engines aren't interested in supporting it, better that there be fewer features that are interoperably supported than more features that aren't.  People should be able to write a page that works in Gecko and have it work in other browsers too.  Having to manually convert lengths or percentages to pixels, or use translate() instead of matrix(), is a lot less annoying to authors than having pages work differently in different browsers.

Is there evidence that authors use this feature?  Are there good use-cases?  I'd think most authors would use translate() and so on, not matrix().  If you're hard-core enough to use matrix(), you're probably able to work around the fact that it only accepts pixels without too much trouble.  If the feature doesn't have clear value, it's going to be a lot easier for Gecko to just drop it than to persuade all the other implementations to add it.

Anyway, I filed a spec bug:

See also bug 719446.
Comment 4 Aryeh Gregor (:ayg) (working until November 1) 2012-02-08 08:37:39 PST
The spec bug was resolved WONTFIX.
Comment 5 Aryeh Gregor (:ayg) (working until November 1) 2012-02-09 07:37:05 PST
(In reply to David Baron [:dbaron] (busy through 12 Feb) from comment #1)
> We should change this in the same release that we unprefix.

Why not now?
Comment 6 Aryeh Gregor (:ayg) (working until November 1) 2012-02-13 08:31:13 PST
Created attachment 596692 [details] [diff] [review]
Patch v1

Not requesting review for now because of comment 1, but it's still worth having a patch ready.  Honestly, we should have unprefixed like a year ago anyway . . .
Comment 7 Mozilla RelEng Bot 2012-02-13 08:36:49 PST
Autoland Patchset:
	Patches: 596692
	Branch: mozilla-central => try
Try run started, revision 9e63d34df962. To cancel or monitor the job, see:
Comment 8 Mozilla RelEng Bot 2012-02-13 15:00:30 PST
Try run for 9e63d34df962 is complete.
Detailed breakdown of the results available here:
Results (out of 211 total builds):
    success: 179
    warnings: 18
    failure: 14
Builds (or logs if builds failed) available at:
Comment 9 Aryeh Gregor (:ayg) (working until November 1) 2012-06-26 07:40:58 PDT
Created attachment 636709 [details] [diff] [review]
Patch v2, rebased

Applies cleanly again.  We're looking to unprefix transforms soon, so I think this should be reviewable -- or should we hold off till 17?

Comment 10 David Baron :dbaron: ⌚️UTC+9 (busy until November 7) 2012-07-10 16:30:00 PDT
Comment on attachment 636709 [details] [diff] [review]
Patch v2, rebased

>diff --git a/layout/reftests/forms/progress/transformations-ref.html b/layout/reftests/forms/progress/transformations-ref.html
>--- a/layout/reftests/forms/progress/transformations-ref.html
>+++ b/layout/reftests/forms/progress/transformations-ref.html
>@@ -1,14 +1,14 @@
> <!DOCTYPE html>
> <html>
>   <link rel='stylesheet' type='text/css' href='style.css'>
>   <style>
>     body > div:nth-child(1) { -moz-transform: matrix(1, -0.2, 0, 1, 0, 0); }
>-    body > div:nth-child(2) { -moz-transform: matrix(1, 0, 0.6, 1, 15em, 0); }
>+    body > div:nth-child(2) { -moz-transform: matrix(1, 0, 0.6, 1, 240, 0); }

I'd suggest making this a matrix(1, 0, 0.6, 1, 0, 0) combined with a translateX(15em), in the correct order (whichever that is).

>diff --git a/layout/reftests/forms/progress/transformations.html b/layout/reftests/forms/progress/transformations.html

Same here.

And I do want this to land for 16 along with the unprefixing.  Thanks for doing this.

Comment 11 Aryeh Gregor (:ayg) (working until November 1) 2012-07-11 00:52:44 PDT
Comment 12 Ed Morley (Away 28th Oct -> 6th Nov) [:emorley] 2012-07-11 09:30:43 PDT

Note You need to log in before you can comment on or make changes to this bug.