Last Comment Bug 773500 - nsStyleAnimation checks for common units of nsCSSValueTriplet are screwy
: nsStyleAnimation checks for common units of nsCSSValueTriplet are screwy
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla17
Assigned To: David Zbarsky (:dzbarsky)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-12 16:29 PDT by David Zbarsky (:dzbarsky)
Modified: 2012-07-22 10:40 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (2.27 KB, patch)
2012-07-12 16:29 PDT, David Zbarsky (:dzbarsky)
dbaron: review-
Details | Diff | Review
Patch (2.08 KB, patch)
2012-07-12 22:02 PDT, David Zbarsky (:dzbarsky)
dbaron: review+
Details | Diff | Review
Followup (5.17 KB, patch)
2012-07-13 14:45 PDT, David Zbarsky (:dzbarsky)
dbaron: review+
Details | Diff | Review

Description David Zbarsky (:dzbarsky) 2012-07-12 16:29:04 PDT
Created attachment 641646 [details] [diff] [review]
Patch
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-07-12 20:06:50 PDT
Comment on attachment 641646 [details] [diff] [review]
Patch

I don't think ValueTriplet needs a URL check at all, and I suspect that ValuePair does in fact only need the URL check on [0] -- it's needed only for SVG paint servers.

But ValueTriplet should check against null for all three values -- and that check should be fixed in ComputeDistance in addition to AddWeighted.

(I remember pointing this out in another bug somewhere.)


This has come up before; not sure how we still managed not to fix it.  It would be great if you could write a revised patch.
Comment 2 David Zbarsky (:dzbarsky) 2012-07-12 22:02:11 PDT
Created attachment 641738 [details] [diff] [review]
Patch

Yeah, I'm positive I've seen this issue discussed before.
Comment 3 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-07-12 22:46:33 PDT
Comment on attachment 641738 [details] [diff] [review]
Patch

r=dbaron
Comment 4 David Zbarsky (:dzbarsky) 2012-07-13 11:51:04 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e62e0d2137b6
Comment 5 :Ehsan Akhgari (busy, don't ask for review please) 2012-07-13 13:04:15 PDT
I backed this out because of mochitest-4 bustage.

https://hg.mozilla.org/integration/mozilla-inbound/rev/cfa35c4507fd

Example failure log: https://tbpl.mozilla.org/php/getParsedLog.php?id=13502718&tree=Mozilla-Inbound&full=1
Comment 6 David Zbarsky (:dzbarsky) 2012-07-13 14:45:20 PDT
Created attachment 642061 [details] [diff] [review]
Followup

If we're going to null-check the zvalue, we should avoid setting it to null.
This change was already part of my transform-origin patch.
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-07-21 16:10:23 PDT
Comment on attachment 642061 [details] [diff] [review]
Followup

>From: David Zbarsky <dzbarsky@gmail.com>
>
>try: -b do

How about:

Store z-component of transform-origin as null rather than 0 when it is omitted.

r=dbaron with that



This should land before the other patch, I think.  Also, when you reland the other patch, don't mention nsCSSValuePair in the summary since the patch doesn't change it.

Note You need to log in before you can comment on or make changes to this bug.