Closed Bug 1264238 Opened 5 years ago Closed 5 years ago
CSSProp List .h entries
From bug 1261754 comment 22. The entries in nsCSSPropList.h are kind of sorted, but not really. It would be nice to keep them sorted.
I think the intention was to have a CSS_PROP_STUB_NOT_CSS entry per property #ifndef CSS_PROP_LIST_EXCLUDE_INTERNAL, but it has been a while since that's been the case. We don't use CSS_PROP_STUB_NOT_CSS anywhere, so let's just remove it.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8740873 - Flags: review?(dholbert)
I decided to keep the entries sorted regardless of #ifdefs around them, so a couple of #ifdef blocks have been broken up.
Attachment #8740874 - Flags: review?(dholbert)
Attachment #8740873 - Flags: review?(dholbert) → review+
Comment on attachment 8740874 [details] [diff] [review] Part 2: Sort nsCSSPropList.h entries. r=me. One opportunity for simplification, around the "mask" / "MOZ_ENABLE_MASK_AS_SHORTHAND" stuff: >+++ b/layout/style/nsCSSPropList.h [...] >+#ifdef MOZ_ENABLE_MASK_AS_SHORTHAND >+CSS_PROP_SHORTHAND( >+ mask, >+ mask, >+ Mask, >+ CSS_PROPERTY_PARSE_FUNCTION, >+ "") >+#else >+CSS_PROP_SVGRESET( >+ mask, [...] >+#endif // MOZ_ENABLE_MASK_AS_SHORTHAND >+#ifdef MOZ_ENABLE_MASK_AS_SHORTHAND >+CSS_PROP_SVGRESET( >+ mask-clip, [...] >+ kImageLayerSizeKTable, >+ CSS_PROP_NO_OFFSET, >+ eStyleAnimType_Custom) >+#endif // MOZ_ENABLE_MASK_AS_SHORTHAND If you like, you could simplify this a bit (collapsing the two separate #if checks into one) if you reverse the polarity of the first check. i.e.: #ifndef MOZ_ENABLE_MASK_AS_SHORTHAND CSS_PROP_SVGRESET( mask, [...] #else CSS_PROP_SHORTHAND( mask, [...] [...all mask-* subproperties go here, except mask-type...] #endif // MOZ_ENABLE_MASK_AS_SHORTHAND I'll grant that it's a bit counterintuitive to use "#ifndef...#else", but it makes this block as a whole much clearer IMO, since now the "mask" shorthand would get to actually be grouped with all of its subproperties, both visually and from an #if-scope perspective. (And it's still alphabetically sorted by property name; just the two ways of representing "mask" itself have been swapped.)
Attachment #8740874 - Flags: review?(dholbert) → review+
(In reply to Daniel Holbert [:dholbert] from comment #4) > If you like, you could simplify this a bit (collapsing the two separate #if > checks into one) if you reverse the polarity of the first check. Nice idea, I've done this.
You need to log in before you can comment on or make changes to this bug.