Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS minifier transforms translate3d(0,0,0) to translate(0), breaking layouts in Safari #2057

Closed
stephband opened this issue Feb 25, 2022 · 8 comments

Comments

@stephband
Copy link

ESBuild's CSS minifier transforms translate3d(0,0,0) to translate(0). Unfortunately in Safari translate3d(0,0,0) is a useful hack, forcing it to respect z-index layering correctly, where translate(0) does not have the same effect.

@stephband stephband changed the title CSS minifier transforms translate3d(0,0,0) to translate(0), breaking Safari CSS minifier transforms translate3d(0,0,0) to translate(0), breaking layouts in Safari Feb 25, 2022
@stephband
Copy link
Author

stephband commented Feb 25, 2022

Looking at the code I notice the minifier is doing a whole bunch of things to 3d transforms. The problem with this is as CSS authors we use 3d transforms to deliberately trigger hardware acceleration, as well as to solve rendering problems in Safari. I would really rather ESBuild was not messing with these, as the transformed result is not as intended by the author:

case "translate3d":
// specifies a 3D translation by the vector [tx,ty,tz], with tx,
// ty and tz being the first, second and third translation-value
// parameters respectively.
if n == 5 {
tx, ty, tz := &args[0], &args[2], &args[4]
tx.TurnLengthOrPercentageIntoNumberIfZero()
ty.TurnLengthOrPercentageIntoNumberIfZero()
tz.TurnLengthIntoNumberIfZero()
if ty.IsZero() && tz.IsZero() {
// "translate3d(tx, 0, 0)" => "translate(tx)"
token.Text = "translate"
*token.Children = args[:1]
} else if tx.IsZero() && tz.IsZero() {
// "translate3d(0, ty, 0)" => "translateY(ty)"
token.Text = "translateY"
*token.Children = args[2:3]
} else if tx.IsZero() && ty.IsZero() {
// "translate3d(0, 0, tz)" => "translateZ(tz)"
token.Text = "translateZ"
*token.Children = args[4:]
} else if tz.IsZero() {
// "translate3d(tx, ty, 0)" => "translate(tx, ty)"
token.Text = "translate"
*token.Children = args[:3]
}
}

expectPrintedMangle(t, "a { transform: translate3d(0, 0, 0) }", "a {\n transform: translate(0);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(0%, 0%, 0) }", "a {\n transform: translate(0);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(0px, 0px, 0px) }", "a {\n transform: translate(0);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(1px, 0px, 0px) }", "a {\n transform: translate(1px);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(0px, 1px, 0px) }", "a {\n transform: translateY(1px);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(0px, 0px, 1px) }", "a {\n transform: translateZ(1px);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(1px, 2px, 3px) }", "a {\n transform: translate3d(1px, 2px, 3px);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(1px, 0, 3px) }", "a {\n transform: translate3d(1px, 0, 3px);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(0, 2px, 3px) }", "a {\n transform: translate3d(0, 2px, 3px);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(1px, 2px, 0px) }", "a {\n transform: translate(1px, 2px);\n}\n")
expectPrintedMangle(t, "a { transform: translate3d(40%, 60%, 0px) }", "a {\n transform: translate(40%, 60%);\n}\n")

@evanw
Copy link
Owner

evanw commented Feb 25, 2022

Please provide a small test case that demonstrates a rendering difference.

@yisibl
Copy link
Contributor

yisibl commented Mar 1, 2022

Indeed, this conversion is not safe.
For related background take a look at: https://developpaper.com/what-you-need-to-know-about-the-css-will-change-property/

@evanw
Copy link
Owner

evanw commented Mar 2, 2022

I still need a small test case that demonstrates a rendering difference. A test is necessary to determine which transforms are safe. I have attempted to read browser source code for this but I still can't verify things without a test case. Here's what I found for WebKit:

@stephband
Copy link
Author

stephband commented Mar 2, 2022

Here is a test case:

https://stephen.band/test/css-transform.html

And here is a screenshot of that page, scrolled to half-way down in Safari:

Screenshot 2022-03-03 at 00 10 31

The orange and yellow blocks both have 100% height, relative position, and a greater z-index than the red block, which has 50% height and is stuck to the bottom. Here we see the red block over the top of the yellow. This never happens to the orange block. The yellow block has transform: translate(0) while the orange block has transform: translate3d(0, 0, 0).

This can be observed in Safari desktop and Safari iOS. Note that the effect is sometimes intermittent - Safari does seem to update itself sometimes and render correctly after a moment.

@evanw
Copy link
Owner

evanw commented Mar 4, 2022

Thanks for the test case. I was able to reproduce the bug. I verified that the following transforms trigger the bug in Safari:

  • matrix3d
  • perspective
  • rotate3d
  • rotateX
  • rotateY
  • scale3d
  • scaleZ
  • translate3d
  • translateZ

and the following transforms do not:

  • matrix
  • rotate
  • rotateZ
  • scale
  • scaleX
  • scaleY
  • skew
  • skewX
  • skewY
  • translate
  • translateX
  • translateY

@evanw evanw closed this as completed in 5bad391 Mar 4, 2022
@Primajin
Copy link

Primajin commented Mar 5, 2022

I think this is good (that we no longer transform 3D into 2D) as one can use a 3D transform (often with 0,0,0) to just force the rendering being made by the GPU.

@stephband
Copy link
Author

@evanw Hi. I have now been able to deploy these changes to production and can confirm it fixes all our woes. Thank you very much for your help, and for ESBuild, which rocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants