Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Remove skew() transformation function (leaving skewX() and skewY())

RESOLVED FIXED in mozilla14

Status

()

Core
CSS Parsing and Computation
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ayg, Assigned: ayg)

Tracking

({dev-doc-complete})

Trunk
mozilla14
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

The skew() transform is misleadingly named and it's not clear what it's useful for, if anything:

http://lists.w3.org/Archives/Public/public-fx/2012JanMar/0156.html

Would Gecko be willing to try dropping support for it?  Do we know if any pages are using it?  It's interoperably implemented in all browsers, so it might well not be worth the pain to get rid of it even if it's useless.
The spec was updated to remove skew(): https://www.w3.org/Bugs/Public/show_bug.cgi?id=16300

Do we want to remove support entirely, or just when we unprefix?
I think we should remove support ASAP.
Created attachment 607015 [details] [diff] [review]
Patch v1

I looked for all mentions of "skew" in layout/style/.  Should I look anywhere else?  I'm not sure I got all the tests, but the try server will tell me that . . .
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Attachment #607015 - Flags: review?(dbaron)
Whiteboard: [autoland-try:-u all]

Updated

5 years ago
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]

Comment 4

5 years ago
Autoland Patchset:
	Patches: 607015
	Branch: mozilla-central => try
Patch 607015 could not be applied to mozilla-central.
patching file layout/style/test/property_database.js
Hunk #1 FAILED at 1013
1 out of 1 hunks FAILED -- saving rejects to file layout/style/test/property_database.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

Patch 607015 could not be applied to mozilla-central.
patching file layout/reftests/transform/compound-1-fail.html
Hunk #1 FAILED at 10
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/compound-1-fail.html.rej
patching file layout/reftests/transform/compound-1-ref.html
Hunk #1 FAILED at 17
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/compound-1-ref.html.rej
patching file layout/reftests/transform/compound-1a.html
Hunk #1 FAILED at 14
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/compound-1a.html.rej
patching file layout/reftests/transform/reftest.list
Hunk #1 FAILED at 76
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/reftest.list.rej
unable to find 'layout/reftests/transform/skew-1-ref.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-1-ref.html.rej
unable to find 'layout/reftests/transform/skew-1a.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-1a.html.rej
unable to find 'layout/reftests/transform/skew-1b.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-1b.html.rej
unable to find 'layout/reftests/transform/skew-2-ref.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-2-ref.html.rej
unable to find 'layout/reftests/transform/skew-2a.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-2a.html.rej
patching file layout/style/nsCSSKeywordList.h
Hunk #1 FAILED at 413
1 out of 1 hunks FAILED -- saving rejects to file layout/style/nsCSSKeywordList.h.rej
patching file layout/style/nsCSSParser.cpp
Hunk #1 FAILED at 7929
1 out of 1 hunks FAILED -- saving rejects to file layout/style/nsCSSParser.cpp.rej
patching file layout/style/nsStyleAnimation.cpp
Hunk #1 FAILED at 1005
Hunk #2 FAILED at 1108
Hunk #3 FAILED at 1523
Hunk #4 FAILED at 1564
Hunk #5 FAILED at 2024
5 out of 5 hunks FAILED -- saving rejects to file layout/style/nsStyleAnimation.cpp.rej
patching file layout/style/nsStyleTransformMatrix.cpp
Hunk #1 FAILED at 416
Hunk #2 FAILED at 590
2 out of 2 hunks FAILED -- saving rejects to file layout/style/nsStyleTransformMatrix.cpp.rej
patching file layout/style/test/property_database.js
Hunk #1 FAILED at 1013
1 out of 1 hunks FAILED -- saving rejects to file layout/style/test/property_database.js.rej
patching file layout/style/test/test_bug721136.html
Hunk #1 FAILED at 18
1 out of 1 hunks FAILED -- saving rejects to file layout/style/test/test_bug721136.html.rej
patching file layout/style/test/test_transitions_per_property.html
Hunk #1 FAILED at 1410
1 out of 1 hunks FAILED -- saving rejects to file layout/style/test/test_transitions_per_property.html.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

Patchset could not be applied and pushed.

Updated

5 years ago
Whiteboard: [autoland-in-queue]
Comment on attachment 607015 [details] [diff] [review]
Patch v1

>-      // It would probably be nicer to animate skew in tangent space
>-      // rather than angle space.  However, it's easy to specify
>-      // skews with infinite tangents, and behavior changes pretty
>-      // drastically when crossing such skews (since the direction of
>-      // animation flips), so interop is probably more important here.

This comment (in nsStyleAnimation.cpp) still applies to skewx and skewy.  Leave it.

>@@ -2025,18 +1944,17 @@ nsStyleAnimation::AddWeighted(nsCSSPrope
>       return true;
>     }
>     case eUnit_Transform: {
>       const nsCSSValueList *list1 = aValue1.GetCSSValueListValue();
>       const nsCSSValueList *list2 = aValue2.GetCSSValueListValue();
> 
>       // We want to avoid the matrix decomposition when we can, since
>       // avoiding it can produce better results both for compound
>-      // transforms and for skew and skewY (see below).  We can do this
>-      // in two cases:
>+      // transforms and for skewY (see below).  We can do this in two cases:

Wrap "cases:" to the next line to match the existing wrapping (which is likely tw=72).

r=dbaron with those fixed
Attachment #607015 - Flags: review?(dbaron) → review+
Created attachment 607182 [details] [diff] [review]
Patch v2

(In reply to David Baron [:dbaron] from comment #5)
> Wrap "cases:" to the next line to match the existing wrapping (which is
> likely tw=72).

Is tw=72 standard at Mozilla?  I generally use tw=79.  All I found is <https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide#Line_length>, which is somewhat nonspecific.
Attachment #607015 - Attachment is obsolete: true
Attachment #607182 - Flags: review+
Whiteboard: [autoland-try:-u all]

Updated

5 years ago
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]

Comment 7

5 years ago
Autoland Patchset:
	Patches: 607182
	Branch: mozilla-central => try
Patch 607182 could not be applied to mozilla-central.
patching file layout/style/test/property_database.js
Hunk #1 FAILED at 1013
1 out of 1 hunks FAILED -- saving rejects to file layout/style/test/property_database.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

Patch 607182 could not be applied to mozilla-central.
patching file layout/reftests/transform/compound-1-fail.html
Hunk #1 FAILED at 10
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/compound-1-fail.html.rej
patching file layout/reftests/transform/compound-1-ref.html
Hunk #1 FAILED at 17
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/compound-1-ref.html.rej
patching file layout/reftests/transform/compound-1a.html
Hunk #1 FAILED at 14
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/compound-1a.html.rej
patching file layout/reftests/transform/reftest.list
Hunk #1 FAILED at 76
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/reftest.list.rej
unable to find 'layout/reftests/transform/skew-1-ref.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-1-ref.html.rej
unable to find 'layout/reftests/transform/skew-1a.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-1a.html.rej
unable to find 'layout/reftests/transform/skew-1b.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-1b.html.rej
unable to find 'layout/reftests/transform/skew-2-ref.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-2-ref.html.rej
unable to find 'layout/reftests/transform/skew-2a.html' for patching
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/transform/skew-2a.html.rej
patching file layout/style/nsCSSKeywordList.h
Hunk #1 FAILED at 413
1 out of 1 hunks FAILED -- saving rejects to file layout/style/nsCSSKeywordList.h.rej
patching file layout/style/nsCSSParser.cpp
Hunk #1 FAILED at 7929
1 out of 1 hunks FAILED -- saving rejects to file layout/style/nsCSSParser.cpp.rej
patching file layout/style/nsStyleAnimation.cpp
Hunk #1 FAILED at 1005
Hunk #2 FAILED at 1108
Hunk #3 FAILED at 1523
Hunk #4 FAILED at 1569
Hunk #5 FAILED at 2024
5 out of 5 hunks FAILED -- saving rejects to file layout/style/nsStyleAnimation.cpp.rej
patching file layout/style/nsStyleTransformMatrix.cpp
Hunk #1 FAILED at 416
Hunk #2 FAILED at 590
2 out of 2 hunks FAILED -- saving rejects to file layout/style/nsStyleTransformMatrix.cpp.rej
patching file layout/style/test/property_database.js
Hunk #1 FAILED at 1013
1 out of 1 hunks FAILED -- saving rejects to file layout/style/test/property_database.js.rej
patching file layout/style/test/test_bug721136.html
Hunk #1 FAILED at 18
1 out of 1 hunks FAILED -- saving rejects to file layout/style/test/test_bug721136.html.rej
patching file layout/style/test/test_transitions_per_property.html
Hunk #1 FAILED at 1410
1 out of 1 hunks FAILED -- saving rejects to file layout/style/test/test_transitions_per_property.html.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir

Patchset could not be applied and pushed.

Updated

5 years ago
Whiteboard: [autoland-in-queue]
Created attachment 607192 [details] [diff] [review]
Patch v3

Oops -- it was conflicting with the bug 719054 patch I have applied locally.  I should make sure any patches I submit are bottommost before I post them.
Attachment #607182 - Attachment is obsolete: true
Attachment #607192 - Flags: review+
Whiteboard: [autoland-try:-u all]

Updated

5 years ago
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]

Comment 9

5 years ago
Autoland Patchset:
	Patches: 607192
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=04c24b87c645
Try run started, revision 04c24b87c645. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=04c24b87c645

Comment 10

5 years ago
Try run for 04c24b87c645 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=04c24b87c645
Results (out of 217 total builds):
    exception: 2
    success: 159
    warnings: 40
    failure: 14
    other: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-04c24b87c645

Updated

5 years ago
Whiteboard: [autoland-in-queue]
Created attachment 607617 [details] [diff] [review]
Patch v4

Fixed test failures in test_bug435293-skew.html.  Otherwise the same as the last version.  Will re-request review when I get a green try run.
Attachment #607192 - Attachment is obsolete: true
Whiteboard: [autoland-try:-u all]

Updated

5 years ago
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]

Comment 12

5 years ago
Autoland Patchset:
	Patches: 607617
	Branch: mozilla-central => try
An error occurred while cloning https://hg.mozilla.org/mozilla-central.

Updated

5 years ago
Whiteboard: [autoland-in-queue]
Whiteboard: [autoland-try:-u all]

Updated

5 years ago
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]

Comment 13

5 years ago
Autoland Patchset:
	Patches: 607617
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=6f350f60aa0f
Try run started, revision 6f350f60aa0f. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=6f350f60aa0f

Comment 14

5 years ago
Try run for ce075b94be45 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=ce075b94be45
Results (out of 217 total builds):
    success: 175
    warnings: 28
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-ce075b94be45

Comment 15

5 years ago
Try run for 6f350f60aa0f is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6f350f60aa0f
Results (out of 219 total builds):
    exception: 1
    success: 178
    warnings: 26
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-6f350f60aa0f
 Timed out after 12 hours without completing.

Updated

5 years ago
Whiteboard: [autoland-in-queue]
Comment on attachment 607617 [details] [diff] [review]
Patch v4

(In reply to Aryeh Gregor from comment #11)
> Created attachment 607617 [details] [diff] [review]
> Patch v4
> 
> Fixed test failures in test_bug435293-skew.html.  Otherwise the same as the
> last version.  Will re-request review when I get a green try run.

Okay, re-requesting review for the test changes.
Attachment #607617 - Flags: review?(dbaron)
Comment on attachment 607617 [details] [diff] [review]
Patch v4

r=dbaron
Attachment #607617 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b48e42774f6
Flags: in-testsuite+
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/8b48e42774f6
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Keywords: dev-doc-needed
I've noticed a broken page due to this change.

http://sudweb.fr/2012/ Below the "Inscrivez-vous" box, skew() is used to create an arrow. The source code of this website is available so I can remove it. Just wondering if this is a sign that this change is affecting real world websites.
Updated https://developer.mozilla.org/en/Firefox_14_for_developers and https://developer.mozilla.org/en/CSS/transform .
Keywords: dev-doc-needed → dev-doc-complete

Updated

5 years ago
Depends on: 747637

Updated

5 years ago
Depends on: 771180

Updated

5 years ago
Depends on: 775046

Updated

5 years ago
Depends on: 775710

Updated

5 years ago
Depends on: 775763
(FTR, the patch for bug 747637 accidentally landed with this bug's number: https://hg.mozilla.org/integration/mozilla-inbound/rev/ad1dadae3427 )

Updated

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