1113419 - eUnit_Filter is inconsistently listed before/after eUnit_Shadow in enum definition / switch statements

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

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.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix v1

This makes the ordering consistently: _Dasharray, _Shadow, _Filter (matching the existing ordering in ComputeDistance & AddWeighted.)

Comment 3

[Dirk Schulze]

Comment on attachment 8538853 [details] [diff] [review]
fix v1

Review of attachment 8538853 [details] [diff] [review]:
-----------------------------------------------------------------

The changes look good to me.

Comment 4

[Daniel Holbert [:dholbert]]

Thanks! Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/056b301fd982

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/056b301fd982