57.0a1 (2017-09-06) (64-bit) Mac OS X Also tested back to 54. The problem started in 56. #container { transform-style: preserve-3d; } <div id="container"> <svg...> </div> Found the problem in https://2018.jsconf.asia/. Works fine in Safari, Chrome and Firefox 55 and earlier.

[Tracking Requested - why for this release]: Regression.

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 Firefox: 57.0a1, Build ID 20170907220212 I have tested this issue on Windows 10 x64 and Mac 10.12, with the latest Nightly (57.0a1) build. I have managed to reproduce it using the steps provided in the description. Considering this, using the Mozregression tool, I have found a regression window: Last good revision: 958d2a5d10091401fd5e900e8e063d21940c137e First bad revision: 7f894f791cdf170d788507d0eff30024ce699523 Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=958d2a5d10091401fd5e900e8e063d21940c137e&tochange=7f894f791cdf170d788507d0eff30024ce699523 Jet, can you please look over the pushlog and see if anything stands out?

Thanks Marius! A couple of options in there... I ni?d Matt in bug 1359709, since it looks most likely.

tracking as regression in 56.

Seems pretty likely that was the cause.

It would be useful to have a testcase (preferably simple) attached to the bug.

Matt, would there be an easy fix or it's just too late for FF56?

Too late for 56 unless you think it is a release blocker or would drive a dot release in 56. We could still take a patch for 57. I am not sure how widespread this issue is. If it has a wide user impact, and you come up with a patch in the next couple of days, please let me know.

I doubt this needs to be a release broken, unless we find out that it's much more widespread than it currently appears to be. Seems to be a fairly obscure corner case, so it's unlikely to affect too many sites.

I guess I'm a little confused here. Is it really the case that some transforms shouldn't work on SVG... or is the issue that we're trying to consider the styles on one element on two different frames (one primary, one anonymous)?

I think it's a bit of both :) preserve-3d shouldn't affect SVG transforms (like the one created for the viewBox property in the testcase), so we don't want to return true for Combines3DTransformWithAncestors() when we don't have a CSS 'transform' style on the frame. The manifestation of this is that we ended up with two frames (one primary, one anonymous), both returning true for IsTransformed (the primary because of the CSS transform, the anonymous because of the viewBox transform), and both returned true for Combines3DTransformWithAncestors (since they returned true for IsTransformed, and their flattened tree parent's primary frame has transform-style:preserve-3d). When we did display list building for the inner frame we were incorrectly trying to combine the viewBox transform into the preserve-3d chain and getting incorrect results.

OK, ugh, I guess I didn't realize that we treated what viewBox does as a transform. (Is that the only thing that an "SVG transform" is, or is it also related to a transform presentational attribute?)

Comment on attachment 8910086 [details] [diff] [review] Don't treat SVG transformed frames as being transformed for the purposes of computing Combines3DTransformWithAncestor I'd probably prefer if you change nsIFrame::IsTransformed to just be: return IsCSSTransformed(aStyleDisplay, aEffectSet) || IsSVGTransformed(); rather than having the two functions mostly repeat themselves.

IsSVGTransformed also checks for transforms applied by an <animateMotion>. (It doesn't return true for transforms that come from a transform="" presentation attribute, from my reading.)

Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/70b150570407 Don't treat SVG transformed frames as being transformed for the purposes of computing Combines3DTransformWithAncestors. r=dbaron

Hi Matt, should we uplift this fix to Beta57?

+1 on uplift. We broke the web a little bit with this regression. I found it because the landing page of the biggest JS conf in Asia was broken... and they were totally fine shipping it that way because it worked for all the other browsers. Matt, what's your estimation of the risk of the fix?

Comment on attachment 8910086 [details] [diff] [review] Don't treat SVG transformed frames as being transformed for the purposes of computing Combines3DTransformWithAncestor Approval Request Comment [Feature/Bug causing the regression]: Bug 1359709 [User impact if declined]: Incorrect rendering on some combinations of preserve-3d and svg, including https://2018.jsconf.asia/ [Is this code covered by automated tests?]: Yes, new reftests added. [Has the fix been verified in Nightly?]: By me, yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Changes code specific to preserve-3d *and* SVG, so has a very limited impact. [String changes made/needed]: None.

Comment on attachment 8910086 [details] [diff] [review] Don't treat SVG transformed frames as being transformed for the purposes of computing Combines3DTransformWithAncestor Fix a new regression, taking it. Should be in 57b5

(In reply to Matt Woodrow (:mattwoodrow) from comment #20) > [Is this code covered by automated tests?]: Yes, new reftests added. > [Has the fix been verified in Nightly?]: By me, yes. > [Needs manual test from QE? If yes, steps to reproduce]: No. Setting qe-verify- based on Matt's assessment on manual testing needs and the fact that this fix has automated coverage.