The default bug view has changed. See this FAQ.

Restore skew() support

VERIFIED FIXED in Firefox 15

Status

()

Core
CSS Parsing and Computation
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Alice0775 White, Assigned: ayg)

Tracking

({dev-doc-complete, regression})

14 Branch
mozilla17
dev-doc-complete, regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox14 affected, firefox15+ fixed, firefox16+ verified, firefox17+ verified, firefox-esr10 unaffected)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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
(Reporter)

Comment 1

5 years ago
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
Keywords: dev-doc-needed
Created attachment 647135 [details] [diff] [review]
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.
Attachment #647135 - Flags: review?(dbaron)
Attachment #647135 - Flags: approval-mozilla-beta?
Attachment #647135 - Flags: approval-mozilla-aurora?
W3C bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=18436

Comment 11

5 years ago
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+

Updated

5 years ago
status-firefox14: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → affected
tracking-firefox15: --- → +
tracking-firefox16: --- → +
tracking-firefox17: --- → +
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.
Attachment #647135 - Flags: approval-mozilla-beta?
Attachment #647135 - Flags: approval-mozilla-beta+
Attachment #647135 - Flags: approval-mozilla-aurora?
Attachment #647135 - Flags: approval-mozilla-aurora+
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.
Flags: in-testsuite+
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/ad1dadae3427
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-aurora/rev/79b7cfde1791
https://hg.mozilla.org/releases/mozilla-beta/rev/a70fc44b8c68
status-firefox-esr10: --- → unaffected
status-firefox15: affected → fixed
status-firefox16: affected → fixed
status-firefox17: affected → 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), …)
Keywords: dev-doc-needed → dev-doc-complete
(Reporter)

Updated

5 years ago
Blocks: 783045
Duplicate of this bug: 783045
Keywords: verifyme
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
status-firefox16: fixed → verified
QA Contact: simona.marcu
Created attachment 672306 [details]
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.
(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.
Status: RESOLVED → VERIFIED
status-firefox17: fixed → verified
Keywords: verifyme

Updated

5 years ago
Blocks: 803074
You need to log in before you can comment on or make changes to this bug.