Closed Bug 1351356 Opened 8 years ago Closed 8 years ago

stylo: Support -moz-transform

Categories

(Core :: CSS Parsing and Computation, defect, P1)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: manishearth, Assigned: canova)

References

Details

Attachments

(1 file)

Bug 1351356 - stylo: Update test expectations for -moz-transform
59 bytes, text/x-review-board-request
manishearth
: review+
Details
-moz-transform is implemented as a shorthand of just transform, which is able to parse the Matrix3dPrefixed and MatrixPrefixed variants. These variants are the same as matrix/matrix3d, however they allow for length/percentage/number/calc in the nondiagonal homogeneous components (last two in matrix(), and fourth-from-last to second-last in matrix3d()). I count 520 instances (in nearly 300 reftests) of -moz-transform in our reftests. Most of these probably could be updated to use regular transform; I bet they were written before regular transform existed. Still, worth supporting this behavior unless we feel like it's not necessary anymore. Should be easy to add the appropriate legacy variants to the transform type and have a flag while parsing, which is set for -moz-transform but not transform.
Assignee: nobody → canaltinova
Priority: -- → P1
Oh, also, we'll need to specialize serialization here. Otherwise, property declaration blocks containing `transform` will always serialize as `-moz-transform`. Probably make get_shorthand_appendable_value bail if the shorthand is -moz-transform. Gecko does this, see http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/layout/style/nsCSSProps.cpp#327 . As a side effect this means that you can set `-moz-transform: matrix(1,1,1,1,10%,10%)` and have it serialize to `transform: matrix(1,1,1,1,10%,10%)`, even though the latter doesn't parse. ¯\_(ツ)_/¯ Quick testcase would be: div = document.createElement('div'); div.style.MozTransform = "matrix(1,1,1,1,10%,10%)" div.style.transform; // should give the same matrix div.style.MozTransform; // should give the same matrix div.style.cssText; // "transform: matrix(...)"
Regarding removing -moz-transform entirely, it was introduced in https://hg.mozilla.org/mozilla-central/rev/110a5d25c8f2
What is the different between bug 1351356 and this?
Nothing, they are the same bug :)
Oh, I think Hiro was referring to bug 1341785, which it looks like CJ is working on. Sorry for the duplication? CJ, have you made any progress on that bug? Nazim was interested in working on this, but if you're already partly done then we can just dupe this one to your bug.
Flags: needinfo?(cku)
If Nazim is interest on it, feel free to take it. I was stock in cleaning up my regression until yesterday, so not much progress so far.
Flags: needinfo?(cku)
(In reply to C.J. Ku[:cjku](UTC+8) from comment #6) > If Nazim is interest on it, feel free to take it. I was stock in cleaning up > my regression until yesterday, so not much progress so far. Ok sounds good - I assigned you a few other bugs to look at when you're ready. :-)
Servo side servo/servo#16003 is for this.
Opened a PR in Servo side since they are only Servo changes: https://github.com/servo/servo/pull/16231
Comment on attachment 8859758 [details] Bug 1351356 - stylo: Update test expectations for -moz-transform https://reviewboard.mozilla.org/r/131762/#review134580 ::: layout/style/test/stylo-failures.md:154 (Diff revision 1) > * test_inherit_storage.html `-moz-force-broken-image-icon` [2] > * test_initial_computation.html `-moz-force-broken-image-icon` [4] > * test_initial_storage.html `-moz-force-broken-image-icon` [4] > * test_value_storage.html `-moz-force-broken-image-icon` [4] > * -moz-transform: need different parsing rules servo/servo#16003 > - * test_inherit_computation.html `-moz-transform`: need different parsing rules [2] > + * test_value_storage.html `-moz-transform`: need different parsing rules [72] Please file a followup for this, and mention it here. ::: layout/style/test/stylo-failures.md:155 (Diff revision 1) > * test_initial_computation.html `-moz-force-broken-image-icon` [4] > * test_initial_storage.html `-moz-force-broken-image-icon` [4] > * test_value_storage.html `-moz-force-broken-image-icon` [4] > * -moz-transform: need different parsing rules servo/servo#16003 > - * test_inherit_computation.html `-moz-transform`: need different parsing rules [2] > - * test_inherit_storage.html `transform`: for -moz-transform [3] > + * test_value_storage.html `-moz-transform`: need different parsing rules [72] > + * test_specified_value_serialization.html `bug-721136` [1] space after bug, not dash Also, that bug seems fixed? Why do we fail it?
Attachment #8859758 - Flags: review?(manishearth) → review+
Comment on attachment 8859758 [details] Bug 1351356 - stylo: Update test expectations for -moz-transform https://reviewboard.mozilla.org/r/131762/#review134580 > Please file a followup for this, and mention it here. Filed: bug 1357906 > space after bug, not dash > > Also, that bug seems fixed? Why do we fail it? Actually it's not related to transform, it tries to parse `swash()` and `stylistic()` functions for `font-variant-alternates` property. But they are implemented as keywords, not functions in servo. See https://github.com/servo/servo/issues/15957#issuecomment-293811727
Pushed by manishearth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/9e316c043296 stylo: Update test expectations for -moz-transform r=manishearth
https://hg.mozilla.org/mozilla-central/rev/9e316c043296
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: