Closed Bug 653842 Opened 9 years ago Closed 9 years ago

[css3-transitions][css3-animations] disallow interpolation out of property's range

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla6

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

Attachments

(3 files)

Since bug 575672 (though it was allowed in opt builds before that, incorrectly), we've allowed cubic-bezier() timing functions for transitions (and now animations) that have result values outside the 0-1 range.  This means that transitions or animations could cause values outside of the allowed range for the property, which we only check at parse time (and when we compute calc()).

nsStyleAnimation should enforce the range restrictions on properties.
Should invalid values that result from cubic-bezier() be clamped or ignored?
clamped.  I have patches for this mostly written in my tree.
Comment on attachment 530789 [details] [diff] [review]
patch 1: list more range restrictions in nsCSSPropList.h

This seems fine.  We need to escalate the font-size-adjust thing to the spec, right?

I don't really have any useful thoughts on the MathML props.
Attachment #530789 - Flags: review?(bzbarsky) → review+
Comment on attachment 530790 [details] [diff] [review]
patch 2: merge more-than-zero into one-or-more

r=me
Attachment #530790 - Flags: review?(bzbarsky) → review+
Comment on attachment 530791 [details] [diff] [review]
patch 3: honor range restrictions when interpolating

>+    // TODO: Up to here...

That should go away.

r=me with that.
Attachment #530791 - Flags: review?(bzbarsky) → review+
(In reply to comment #6)
> This seems fine.  We need to escalate the font-size-adjust thing to the
> spec, right?

I already did: http://lists.w3.org/Archives/Public/www-style/2011Apr/0812.html
Comment on attachment 530790 [details] [diff] [review]
patch 2: merge more-than-zero into one-or-more

>diff --git a/layout/style/nsCSSProps.h b/layout/style/nsCSSProps.h
>--- a/layout/style/nsCSSProps.h
>+++ b/layout/style/nsCSSProps.h
>@@ -108,21 +108,18 @@
> PR_STATIC_ASSERT((CSS_PROPERTY_PARSE_PROPERTY_MASK &
>                   CSS_PROPERTY_VALUE_PARSER_FUNCTION) == 0);
> 
> #define CSS_PROPERTY_VALUE_RESTRICTION_MASK       (3<<13)
> // The parser (in particular, CSSParserImpl::ParseSingleValueProperty)
> // should enforce that the value of this property must be 0 or larger.
> #define CSS_PROPERTY_VALUE_NONNEGATIVE            (1<<13)
> // The parser (in particular, CSSParserImpl::ParseSingleValueProperty)
>-// should enforce that the value of this property must be greater than 0.
>-#define CSS_PROPERTY_VALUE_POSITIVE_NONZERO       (2<<13)
>-// The parser (in particular, CSSParserImpl::ParseSingleValueProperty)
> // should enforce that the value of this property must be 1 or larger.
>-#define CSS_PROPERTY_VALUE_AT_LEAST_ONE           (3<<13)
>+#define CSS_PROPERTY_VALUE_AT_LEAST_ONE           (2<<13)
> 
> // NOTE: next free bit is (1<<15)

Is this comment still right?
https://hg.mozilla.org/mozilla-central/rev/5752142febe7
https://hg.mozilla.org/mozilla-central/rev/c2613933f08b
https://hg.mozilla.org/mozilla-central/rev/e5fc7dbde8be

My current inclination is not to try to get this in to Firefox 5.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Priority: -- → P3
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.