nsStyleAnimation checks for common units of nsCSSValueTriplet are screwy

RESOLVED FIXED in mozilla17

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dzbarsky, Assigned: dzbarsky)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 641646 [details] [diff] [review]
Patch
Attachment #641646 - Flags: review?(matt.woodrow)
(Assignee)

Updated

6 years ago
Attachment #641646 - Flags: review?(matt.woodrow) → review?(dbaron)
(Assignee)

Updated

6 years ago
Assignee: nobody → dzbarsky
Status: NEW → ASSIGNED
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.
Attachment #641646 - Flags: review?(dbaron) → review-
(Assignee)

Comment 2

6 years ago
Created attachment 641738 [details] [diff] [review]
Patch

Yeah, I'm positive I've seen this issue discussed before.
Attachment #641646 - Attachment is obsolete: true
Attachment #641738 - Flags: review?(dbaron)
Summary: nsStyleAnimation checks for common units of nsCSSValuePair/Triplet are screwy → nsStyleAnimation checks for common units of nsCSSValueTriplet are screwy
(Assignee)

Comment 6

6 years ago
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.
Attachment #642061 - Flags: review?(dbaron)
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.
Attachment #642061 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/mozilla-central/rev/292c6ef7188f
https://hg.mozilla.org/mozilla-central/rev/52471266a657
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.