Closed
Bug 653842
Opened 13 years ago
Closed 13 years ago
[css3-transitions][css3-animations] disallow interpolation out of property's range
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla6
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
Attachments
(3 files)
8.20 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
8.45 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
54.47 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•13 years ago
|
||
Should invalid values that result from cubic-bezier() be clamped or ignored?
Assignee | ||
Comment 2•13 years ago
|
||
clamped. I have patches for this mostly written in my tree.
Assignee | ||
Comment 3•13 years ago
|
||
Attachment #530789 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #530790 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #530791 -
Flags: review?(bzbarsky)
Comment 6•13 years ago
|
||
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 7•13 years ago
|
||
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 8•13 years ago
|
||
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+
Assignee | ||
Comment 9•13 years ago
|
||
(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 10•13 years ago
|
||
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?
Assignee | ||
Comment 11•13 years ago
|
||
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: 13 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.
Description
•