Closed
Bug 1113419
Opened 9 years ago
Closed 9 years ago
eUnit_Filter is inconsistently listed before/after eUnit_Shadow in enum definition / switch statements
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
3.41 KB,
patch
|
krit
:
review+
|
Details | Diff | Splinter Review |
eUnit_Filter is listed between eUnit_Dasharray and eUnit_Shadow in StyleAnimationValue.h: > 222 eUnit_Dasharray, // nsCSSValueList* (never null) > 223 eUnit_Filter, // nsCSSValueList* (may be null) > 224 eUnit_Shadow, // nsCSSValueList* (may be null) > 225 eUnit_Transform, // nsCSSValueList* (never null) https://mxr.mozilla.org/mozilla-central/source/layout/style/StyleAnimationValue.h?rev=156fa5c40afc#222 But in ComputeDistance, we've got it listed after _Shadow: > 761 case eUnit_Shadow: { [...] > 830 } > 831 case eUnit_Filter: > 832 // FIXME: Support paced animations for filter function interpolation. > 833 case eUnit_Transform: { > 834 return false; > 835 } https://mxr.mozilla.org/mozilla-central/source/layout/style/StyleAnimationValue.cpp?rev=983af7fa41f5#831 We should shift it earlier, to keep the order consistent.
Assignee | ||
Comment 1•9 years ago
|
||
Actually, AddWeighted has the same issue -- _Filter comes after _Shadow there as well. It's a smaller change to just reorder the enum list (to make the ComputeDistance/AddWeighted ordering correct), so let's just do that.
Assignee | ||
Updated•9 years ago
|
Summary: "case eUnit_Filter" is at wrong position in StyleAnimationValue::ComputeDistance switch statement → eUnit_Filter is inconsistently listed before/after eUnit_Shadow in enum definition / switch statements
Assignee | ||
Comment 2•9 years ago
|
||
This makes the ordering consistently: _Dasharray, _Shadow, _Filter (matching the existing ordering in ComputeDistance & AddWeighted.)
Comment 3•9 years ago
|
||
Comment on attachment 8538853 [details] [diff] [review] fix v1 Review of attachment 8538853 [details] [diff] [review]: ----------------------------------------------------------------- The changes look good to me.
Attachment #8538853 -
Flags: review?(krit) → review+
Assignee | ||
Comment 4•9 years ago
|
||
Thanks! Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/056b301fd982
Flags: in-testsuite-
Comment 5•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/056b301fd982
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in
before you can comment on or make changes to this bug.
Description
•