style: Tweak background-color and color handling in ignored-colors mode.

We're resetting `color` to the default color when there's a declaration that
applies in order to make stuff like this:

<div style="color: transparent">
  <div style="color: red">
    Red
  </div>
</div>

To not show transparent. But the behavior we want is more like "override with
default color iff there's no other declaration that would set the color from an
user or UA sheet".

This implements that behavior, plus avoids it if we're not inheriting
from transparent, so that stuff like this preserves the behavior from before bug
844349:

<a href="foo">
  <span style="color: red">Should be the red color</span>
</a>

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

components/style/properties/cascade.rs

@@ -333,27 +333,37 @@ where
     context.builder.build()
 }
 
-fn should_ignore_declaration_when_ignoring_document_colors(
+/// How should a declaration behave when ignoring document colors?
-    device: &Device,
+enum DeclarationApplication {
+    /// We should apply the declaration.
+    Apply,
+    /// We should ignore the declaration.
+    Ignore,
+    /// We should apply the following declaration, only if any other declaration
+    /// hasn't set it before.
+    ApplyUnlessOverriden(PropertyDeclaration),
+}
+
+fn application_when_ignoring_colors(
+    builder: &StyleBuilder,
     longhand_id: LonghandId,
     origin: Origin,
-    pseudo: Option<&PseudoElement>,
+    declaration: &PropertyDeclaration,
-    declaration: &mut Cow<PropertyDeclaration>,
+) -> DeclarationApplication {
-) -> bool {
     if !longhand_id.ignored_when_document_colors_disabled() {
-        return false;
+        return DeclarationApplication::Apply;
     }
 
     let is_ua_or_user_rule = matches!(origin, Origin::User | Origin::UserAgent);
     if is_ua_or_user_rule {
-        return false;
+        return DeclarationApplication::Apply;
     }
 
     // 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 pseudo.map_or(false, |p| p.is_color_swatch()) && longhand_id == LonghandId::BackgroundColor {
+    if builder.pseudo.map_or(false, |p| p.is_color_swatch()) && longhand_id == LonghandId::BackgroundColor {
-        return false;
+        return DeclarationApplication::Apply;
     }
 
     // Treat background-color a bit differently.  If the specified color is
@@ -365,30 +375,41 @@ fn should_ignore_declaration_when_ignoring_document_colors(
     // a background image, if we're ignoring document colors).
     // Here we check backplate status to decide if ignoring background-image
     // is the right decision.
-    let (is_background, is_transparent) = match **declaration {
+    match *declaration {
-        PropertyDeclaration::BackgroundColor(ref color) => (true, color.is_transparent()),
+        PropertyDeclaration::BackgroundColor(ref color) => {
-        PropertyDeclaration::Color(ref color) => (false, color.0.is_transparent()),
+            if color.is_transparent() {
+                return DeclarationApplication::Apply;
+            }
+            let color = builder.device.default_background_color();
+            DeclarationApplication::ApplyUnlessOverriden(
+                PropertyDeclaration::BackgroundColor(color.into())
+            )
+        }
+        PropertyDeclaration::Color(ref color) => {
+            if color.0.is_transparent() {
+                return DeclarationApplication::Apply;
+            }
+            if builder.get_parent_inherited_text().clone_color().alpha != 0 {
+                return DeclarationApplication::Ignore;
+            }
+            let color = builder.device.default_color();
+            DeclarationApplication::ApplyUnlessOverriden(
+                PropertyDeclaration::Color(specified::ColorPropertyValue(color.into()))
+            )
+        },
         // In the future, if/when we remove the backplate pref, we can remove this
         // special case along with the 'ignored_when_colors_disabled=True' mako line
         // for the "background-image" property.
         #[cfg(feature = "gecko")]
-        PropertyDeclaration::BackgroundImage(..) => return !static_prefs::pref!("browser.display.permit_backplate"),
+        PropertyDeclaration::BackgroundImage(..) => {
-        _ => return true,
+            if static_prefs::pref!("browser.display.permit_backplate") {
-    };
+                DeclarationApplication::Apply
-
+            } else {
-    if is_transparent {
+                DeclarationApplication::Ignore
-        return false;
+            }
+        },
+        _ => DeclarationApplication::Ignore,
     }
-
-    if is_background {
-        let color = device.default_background_color();
-        *declaration.to_mut() = PropertyDeclaration::BackgroundColor(color.into());
-    } else {
-        let color = device.default_color();
-        *declaration.to_mut() = PropertyDeclaration::Color(specified::ColorPropertyValue(color.into()));
-    }
-
-    false
 }
 
 struct Cascade<'a, 'b: 'a> {
@@ -465,6 +486,8 @@ 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();
 
         for (declaration, origin) in declarations {
             let declaration_id = declaration.id();
@@ -506,21 +529,25 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
                 continue;
             }
 
-            let mut declaration = self.substitute_variables_if_needed(declaration);
+            let declaration = self.substitute_variables_if_needed(declaration);
 
-            // When document colors are disabled, skip properties that are
+            // When document colors are disabled, do special handling of
-            // marked as ignored in that mode, unless they come from a UA or
+            // properties that are marked as ignored in that mode.
-            // user style sheet.
             if ignore_colors {
-                let should_ignore = should_ignore_declaration_when_ignoring_document_colors(
+                let application = application_when_ignoring_colors(
-                    self.context.builder.device,
+                    &self.context.builder,
                     longhand_id,
                     origin,
-                    self.context.builder.pseudo,
+                    &declaration,
-                    &mut declaration,
                 );
-                if should_ignore {
+
-                    continue;
+                match application {
+                    DeclarationApplication::Ignore => continue,
+                    DeclarationApplication::Apply => {},
+                    DeclarationApplication::ApplyUnlessOverriden(decl) => {
+                        declarations_to_apply_unless_overriden.push(decl);
+                        continue;
+                    }
                 }
             }
 
@@ -558,6 +585,20 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
             self.apply_declaration(longhand_id, &*declaration);
         }
 
+        if ignore_colors {
+            for declaration in declarations_to_apply_unless_overriden.iter() {
+                let longhand_id = match declaration.id() {
+                    PropertyDeclarationId::Longhand(id) => id,
+                    PropertyDeclarationId::Custom(..) => unreachable!(),
+                };
+                debug_assert!(!longhand_id.is_logical());
+                if self.seen.contains(longhand_id) {
+                    continue;
+                }
+                self.apply_declaration(longhand_id, declaration);
+            }
+        }
+
         if Phase::is_early() {
             self.fixup_font_stuff();
             self.compute_writing_mode();