style: Respect the cascade properly when in high-contrast mode.

Differential Revision: https://phabricator.services.mozilla.com/D65345

components/style/properties/cascade.rs

@@ -337,94 +337,74 @@ where
     context.builder.build()
 }
 
-/// How should a declaration behave when ignoring document colors?
+/// For ignored colors mode, we sometimes want to do something equivalent to
-enum DeclarationApplication {
+/// "revert-or-initial", where we `revert` for a given origin, but then apply a
-    /// We should apply the declaration.
+/// given initial value if nothing in other origins did override it.
-    Apply,
+///
-    /// We should ignore the declaration.
+/// This is a bit of a clunky way of achieving this.
-    Ignore,
+type DeclarationsToApplyUnlessOverriden = SmallVec::<[PropertyDeclaration; 2]>;
-    /// We should apply the following declaration, only if any other declaration
-    /// hasn't set it before.
-    ApplyUnlessOverriden(PropertyDeclaration),
-}
 
-fn application_when_ignoring_colors(
+fn tweak_when_ignoring_colors(
     builder: &StyleBuilder,
     longhand_id: LonghandId,
     origin: Origin,
-    declaration: &PropertyDeclaration,
+    declaration: &mut Cow<PropertyDeclaration>,
-) -> DeclarationApplication {
+    declarations_to_apply_unless_overriden: &mut DeclarationsToApplyUnlessOverriden,
+) {
     if !longhand_id.ignored_when_document_colors_disabled() {
-        return DeclarationApplication::Apply;
+        return;
     }
 
     let is_ua_or_user_rule = matches!(origin, Origin::User | Origin::UserAgent);
     if is_ua_or_user_rule {
-        return DeclarationApplication::Apply;
+        return;
     }
 
     // Don't override background-color on ::-moz-color-swatch. It is set as an
     // author style (via the style attribute), but it's pretty important for it
     // to show up for obvious reasons :)
     if builder.pseudo.map_or(false, |p| p.is_color_swatch()) && longhand_id == LonghandId::BackgroundColor {
-        return DeclarationApplication::Apply;
+        return;
     }
 
-    // Treat background-color a bit differently.  If the specified color is
+    // A few special-cases ahead.
-    // anything other than a fully transparent color, convert it into the
+    match **declaration {
-    // Device's default background color.
+        // We honor color and background-color: transparent, and
-    // Also: for now, we treat background-image a bit differently, too.
+        // "revert-or-initial" otherwise.
-    // background-image is marked as ignored, but really, we only ignore
-    // it when backplates are disabled (since then text may be unreadable over
-    // a background image, if we're ignoring document colors).
-    // Here we check backplate status to decide if ignoring background-image
-    // is the right decision.
-    match *declaration {
-        // If we've got multiple declarations in the same block, they'll
-        // get overridden at parse time. We should probably adjust this
-        // to turn Ignored decls into `none`. See 1619701
         PropertyDeclaration::BackgroundColor(ref color) => {
             if color.is_transparent() {
-                return DeclarationApplication::Apply;
+                return;
             }
             let color = builder.device.default_background_color();
-            DeclarationApplication::ApplyUnlessOverriden(
+            declarations_to_apply_unless_overriden.push(
                 PropertyDeclaration::BackgroundColor(color.into())
             )
         }
         PropertyDeclaration::Color(ref color) => {
+            // otherwise.
             if color.0.is_transparent() {
-                return DeclarationApplication::Apply;
+                return;
-            }
-            if builder.get_parent_inherited_text().clone_color().alpha != 0 {
-                return DeclarationApplication::Ignore;
             }
             let color = builder.device.default_color();
-            DeclarationApplication::ApplyUnlessOverriden(
+            declarations_to_apply_unless_overriden.push(
                 PropertyDeclaration::Color(specified::ColorPropertyValue(color.into()))
             )
         },
-        // In the future, if/when we remove the backplate pref, we can remove this
+        // We honor url background-images if backplating.
-        // special case along with the 'ignored_when_colors_disabled=True' mako line
-        // for the "background-image" property.
         #[cfg(feature = "gecko")]
         PropertyDeclaration::BackgroundImage(ref bkg) => {
             use crate::values::generics::image::Image;
-            // We should only allow images to be rendered in HCM if the backplate pref
-            // is enabled, and if all the images applied to the background are from URLs.
-            // If one or more background images aren't from URL's (ie. gradients)
-            // we should ignore all background-image styling.
             if static_prefs::pref!("browser.display.permit_backplate") {
                 if bkg.0.iter().all(|image| matches!(*image, Image::Url(..))) {
-                    return DeclarationApplication::Apply;
+                    return;
                 }
-                return DeclarationApplication::Ignore;
-            } else {
-                return DeclarationApplication::Ignore;
             }
         },
-        _ => DeclarationApplication::Ignore,
+        _ => {},
     }
+
+    *declaration.to_mut() = PropertyDeclaration::css_wide_keyword(longhand_id, CSSWideKeyword::Revert);
+
 }
 
 struct Cascade<'a, 'b: 'a> {
@@ -502,7 +482,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
 
         let ignore_colors = !self.context.builder.device.use_document_colors();
         let mut declarations_to_apply_unless_overriden =
-            SmallVec::<[PropertyDeclaration; 2]>::new();
+            DeclarationsToApplyUnlessOverriden::new();
 
         for (declaration, origin) in declarations {
             let declaration_id = declaration.id();
@@ -544,26 +524,23 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 continue;
             }
 
-            let declaration = self.substitute_variables_if_needed(declaration);
+            let mut declaration = self.substitute_variables_if_needed(declaration);
 
             // When document colors are disabled, do special handling of
             // properties that are marked as ignored in that mode.
             if ignore_colors {
-                let application = application_when_ignoring_colors(
+                tweak_when_ignoring_colors(
                     &self.context.builder,
                     longhand_id,
                     origin,
-                    &declaration,
+                    &mut declaration,
+                    &mut declarations_to_apply_unless_overriden,
+                );
+                debug_assert_eq!(
+                    declaration.id(),
+                    PropertyDeclarationId::Longhand(longhand_id),
+                    "Shouldn't change the declaration id!",
                 );
-
-                match application {
-                    DeclarationApplication::Ignore => continue,
-                    DeclarationApplication::Apply => {},
-                    DeclarationApplication::ApplyUnlessOverriden(decl) => {
-                        declarations_to_apply_unless_overriden.push(decl);
-                        continue;
-                    }
-                }
             }
 
             let css_wide_keyword = declaration.get_css_wide_keyword();

components/style/properties/properties.mako.rs

@@ -1590,10 +1590,7 @@ impl UnparsedValue {
             } else {
                 CSSWideKeyword::Initial
             };
-            PropertyDeclaration::CSSWideKeyword(WideKeywordDeclaration {
+            PropertyDeclaration::css_wide_keyword(longhand_id, keyword)
-                id: longhand_id,
-                keyword,
-            })
         };
 
         let css = match crate::custom_properties::substitute(
@@ -1630,10 +1627,7 @@ impl UnparsedValue {
         let mut input = Parser::new(&mut input);
         input.skip_whitespace();  // Unnecessary for correctness, but may help try() rewind less.
         if let Ok(keyword) = input.try(CSSWideKeyword::parse) {
-            return PropertyDeclaration::CSSWideKeyword(WideKeywordDeclaration {
+            return PropertyDeclaration::css_wide_keyword(longhand_id, keyword);
-                id: longhand_id,
-                keyword,
-            });
         }
 
         let declaration = input.parse_entirely(|input| {
@@ -2239,6 +2233,12 @@ impl PropertyDeclaration {
         }
     }
 
+    /// Returns a CSS-wide keyword declaration for a given property.
+    #[inline]
+    pub fn css_wide_keyword(id: LonghandId, keyword: CSSWideKeyword) -> Self {
+        Self::CSSWideKeyword(WideKeywordDeclaration { id, keyword })
+    }
+
     /// Returns a CSS-wide keyword if the declaration's value is one.
     #[inline]
     pub fn get_css_wide_keyword(&self) -> Option<CSSWideKeyword> {
@@ -2367,9 +2367,7 @@ impl PropertyDeclaration {
             PropertyId::Longhand(id) => {
                 input.skip_whitespace();  // Unnecessary for correctness, but may help try() rewind less.
                 input.try(CSSWideKeyword::parse).map(|keyword| {
-                    PropertyDeclaration::CSSWideKeyword(
+                    PropertyDeclaration::css_wide_keyword(id, keyword)
-                        WideKeywordDeclaration { id, keyword },
-                    )
                 }).or_else(|()| {
                     input.look_for_var_or_env_functions();
                     input.parse_entirely(|input| id.parse_value(context, input))
@@ -2403,12 +2401,7 @@ impl PropertyDeclaration {
                         declarations.all_shorthand = AllShorthand::CSSWideKeyword(keyword)
                     } else {
                         for longhand in id.longhands() {
-                            declarations.push(PropertyDeclaration::CSSWideKeyword(
+                            declarations.push(PropertyDeclaration::css_wide_keyword(longhand, keyword));
-                                WideKeywordDeclaration {
-                                    id: longhand,
-                                    keyword,
-                                },
-                            ))
                         }
                     }
                 } else {
@@ -2550,12 +2543,7 @@ impl<'a> Iterator for AllShorthandDeclarationIterator<'a> {
         match *self.all_shorthand {
             AllShorthand::NotSet => None,
             AllShorthand::CSSWideKeyword(ref keyword) => {
-                Some(PropertyDeclaration::CSSWideKeyword(
+                Some(PropertyDeclaration::css_wide_keyword(self.longhands.next()?, *keyword))
-                    WideKeywordDeclaration {
-                        id: self.longhands.next()?,
-                        keyword: *keyword
-                    }
-                ))
             }
             AllShorthand::WithVariables(ref unparsed) => {
                 Some(PropertyDeclaration::WithVariables(