Skip to content

Commit

Permalink
style: Tweak background-color and color handling in ignored-colors mode.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
emilio committed Feb 12, 2020
1 parent 017d6f0 commit 13d12d0
Showing 1 changed file with 79 additions and 38 deletions.
117 changes: 79 additions & 38 deletions components/style/properties/cascade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,27 +333,37 @@ where
context.builder.build()
}

fn should_ignore_declaration_when_ignoring_document_colors(
device: &Device,
/// How should a declaration behave when ignoring document colors?
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: &mut Cow<PropertyDeclaration>,
) -> bool {
declaration: &PropertyDeclaration,
) -> DeclarationApplication {
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 {
return false;
if builder.pseudo.map_or(false, |p| p.is_color_swatch()) && longhand_id == LonghandId::BackgroundColor {
return DeclarationApplication::Apply;
}

// Treat background-color a bit differently. If the specified color is
Expand All @@ -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 {
PropertyDeclaration::BackgroundColor(ref color) => (true, color.is_transparent()),
PropertyDeclaration::Color(ref color) => (false, color.0.is_transparent()),
match *declaration {
PropertyDeclaration::BackgroundColor(ref color) => {
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"),
_ => return true,
};

if is_transparent {
return false;
}

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()));
PropertyDeclaration::BackgroundImage(..) => {
if static_prefs::pref!("browser.display.permit_backplate") {
DeclarationApplication::Apply
} else {
DeclarationApplication::Ignore
}
},
_ => DeclarationApplication::Ignore,
}

false
}

struct Cascade<'a, 'b: 'a> {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
// marked as ignored in that mode, unless they come from a UA or
// user style sheet.
// When document colors are disabled, do special handling of
// properties that are marked as ignored in that mode.
if ignore_colors {
let should_ignore = should_ignore_declaration_when_ignoring_document_colors(
self.context.builder.device,
let application = application_when_ignoring_colors(
&self.context.builder,
longhand_id,
origin,
self.context.builder.pseudo,
&mut declaration,
&declaration,
);
if should_ignore {
continue;

match application {
DeclarationApplication::Ignore => continue,
DeclarationApplication::Apply => {},
DeclarationApplication::ApplyUnlessOverriden(decl) => {
declarations_to_apply_unless_overriden.push(decl);
continue;
}
}
}

Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 13d12d0

Please sign in to comment.