747637 - Restore skew() support

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Alice0775 White]

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

Comment 1

[Alice0775 White]

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.

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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?

Comment 3

[Boris Zbarsky [:bzbarsky]]

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.

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

David Baron and I agreed that there's too much breakage -- we need to back out bug 734953 and re-add support for skew().

Comment 9

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Backout patch

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.

Comment 10

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

W3C bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=18436

Comment 11

[Steve Kwan]

Thank you so much for taking the initiative on this.  There are definitely some of us who rely on this feature quite extensively.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 13

[Alex Keybl [:akeybl]]

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.

Comment 14

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

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.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ad1dadae3427

Comment 16

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/79b7cfde1791
https://hg.mozilla.org/releases/mozilla-beta/rev/a70fc44b8c68

Comment 17

[Jean-Yves Perrier [:teoli]]

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), …)

Comment 19

[Simona Badau, Desktop QA]

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

Comment 20

[Simona Badau, Desktop QA]

Attachment: Screenshot with the issue

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.

Comment 21

[Alex Keybl [:akeybl]]

(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.

Comment 22

[u279076]

Calling this verified fixed based on comment 20 and 21. Simona, please clone this bug with your testcase.