Closed Bug 747637 Opened 11 years ago Closed 10 years ago
Restore skew() support
Build Identifier: http://hg.mozilla.org/mozilla-central/rev/22bfdebf5cae Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120420 Firefox/14.0a1 ID:20120420030653 Rendering is broken due to Remove skew() transformation function
http://ie.microsoft.com/testdrive/Graphics/hands-on-css3/hands-on_2d-transforms.htm Select skew(θx,θy)of "Append another transform function"dropdown And change degree by slider Nothing happen in Firefox. http://www.zachstronaut.com/lab/isocube.html Rendering is broken in Firefox.
This seems like evangelism to me -- we want to contact MS and get them to take down that demo. skew() is no longer in the spec. How do we proceed in cases like this? Do we have specific contacts at MS that we can call?
Summary: Rendering is broken due to Remove skew() transformation function → Rendering is broken in MS transforms demo due to Remove skew() transformation function
Contacting MS won't fix the other two testcases linked from this bug. So while I can do that, it's not clear that I should. Frankly, the way we usually proceed in cases like this is by readding back to the spec the thing that's being used on the web already that got removed from the spec. Imo.
David Baron and I agreed that there's too much breakage -- we need to back out bug 734953 and re-add support for skew().
Assignee: nobody → ayg
Status: NEW → ASSIGNED
OS: Windows 7 → All
Hardware: x86_64 → All
Summary: Rendering is broken in MS transforms demo due to Remove skew() transformation function → Restore skew() support
Duplicate of this bug: 771180
Duplicate of this bug: 775046
Duplicate of this bug: 775710
Duplicate of this bug: 775763
Try: https://tbpl.mozilla.org/?tree=Try&rev=18be344c0eb5 The backout was almost perfectly clean. [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 734953. All browsers supported the skew() function in their CSS transforms implementations, but the spec editors agreed that it was useless and we should try to drop it. All Firefox versions < 14 support it, and so do all versions of competing browsers (if they support transforms at all). User impact if declined: We've received reports of several sites that display incorrectly because they depend on the removed feature. There are probably more. David Baron and I agreed that the compat impact is not worth dropping this feature; we intend to keep it indefinitely and get the spec updated to require it again. All other browsers support skew() and always have, so any site that breaks because of this bug will work fine in all other browsers. Testing completed (on m-c, etc.): This is a backout patch to revert to pre-14 behavior. Risk to taking this patch (and alternatives if risky): Minimal. The feature is quite isolated, so it's unlikely that we'd have substantial breakage for taking it. Almost all the code to implement the feature was always there anyway for the skewX() and skewY() functions, which we never dropped support for, so we're more or less just readding parser support. String or UUID changes made by this patch: None.
Thank you so much for taking the initiative on this. There are definitely some of us who rely on this feature quite extensively.
Comment on attachment 647135 [details] [diff] [review] Backout patch In property_database.js, you should convert the -moz-max( to max( to match the unprefixing of calc() -- we removed max() support, but in case we ever add it back, I suppose. (I spent a little time checking the nsStyleAnimation stuff in light of bug 769193, but I think it's right.) r=dbaron with that
Attachment #647135 - Flags: review?(dbaron) → review+
Comment on attachment 647135 [details] [diff] [review] Backout patch [Triage Comment] Given the minimal user benefit to leaving this feature removed, let's re-add for 15 and up.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad1dadae3427 (This had the wrong bug number in the commit message.) I could try to push to aurora/beta tomorrow morning, when I have time to watch the tree. Otherwise, someone else will have to do it. Probably the original backout will have fewer conflicts on aurora/beta, so the patch I attached here likely won't apply, but re-backing out the original patch should work fine.
Target Milestone: --- → mozilla17
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
A note has been added to https://developer.mozilla.org/en-US/docs/Firefox_15_for_developers and https://developer.mozilla.org/en-US/docs/CSS/transform Note that everywhere in the MDN we tell the Web devs that this function is non-standard, shouldn't be used and give advice on how to fix it (like skew(x) ->skeyX(x), …)
Verified that the skew() support is restored on Firefox 16 beta 5 - there are no rendering issues on the web sites provided in the URL, Comment 1 and the duplicate bugs, except for http://www.zachstronaut.com/lab/isocube.html where the rendering is broken on several browsers (Chrome, Opera and Safari). Mozilla/5.0 (Windows NT 6.1; rv:16.0) Gecko/20100101 Firefox/16.0 Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/20100101 Firefox/16.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:16.0) Gecko/20100101 Firefox/16.0
When verifying this fix on Firefox 17 beta 1, I observed some rendering issues that are not reproducible on Firefox 16 beta 6. Steps to reproduce: 1. Navigate to: http://ie.microsoft.com/testdrive/Graphics/hands-on-css3/hands-on_2d-transforms.htm 2. Select skew(θx,θy)of "Append another transform function"dropdown 3. Change degree by slider. Actual results: Rendering issues when changing degree by slider (easily reproducible on Ubuntu 12.04 but also on Mac OS X 10.7 and Windows 7). Please see the attachment for more details. Please let me know if you would prefer me to reopen this bug or file a new one.
(In reply to Simona B [QA] from comment #20) > Please let me know if you would prefer me to reopen this bug or file a new > one. Please clone this bug (to maintain the tracking flag and assignee) with the new STR and screenshot. Or file a new one and nominate for firefox 17 tracking.
Calling this verified fixed based on comment 20 and 21. Simona, please clone this bug with your testcase.
You need to log in before you can comment on or make changes to this bug.