From f70b0e7108a68420840e93938aeb55c0bccca885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 30 May 2023 20:41:21 +0200 Subject: [PATCH] style: Make Canvas/CanvasText and Link colors color-scheme-aware For that, add `.dark` version of the browser.display* prefs that control the light version of these colors. The default for background/foreground colors are taken from the GenericDarkColors used in LookAndFeel. The defaults for links are based on this discussion: https://github.com/whatwg/html/issues/5426#issuecomment-904021675 (So they effectively match Chrome). Whether the dark colors should be exposed in about:preferences (like the light colors are) is TBD. With this patch, we pass all the tests in: /html/semantics/document-metadata/the-meta-element/color-scheme/ Use the colors to paint the default canvas background and the default colors. There are three "regressions", though they are really progressions: we now render the reference as the test expects (before we rendered a light canvas background even for the reference). Apart of these iframe tests (which we should look into, I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1738380), there are three remaining test failures. Two of them are due to `color: initial` not changing based on the color-scheme. Safari also fails these tests, and the thing they're really testing is whether system colors are preserved at computed-value time: https://github.com/w3c/csswg-drafts/issues/3847 Regarding that change, I'm not so sure the trade-offs there are worth it, as that not only complicates interpolation (we wouldn't be able to use system colors in color-mix among others, see https://github.com/w3c/csswg-drafts/issues/5780) plus it changes inheritance behavior in sorta unexpected ways, see: https://github.com/w3c/csswg-drafts/issues/6773 Which I just filed because apparently no browser implements this correctly. So for now will punt on those (keep matching Safari). There's an svg-as-image test: https://searchfox.org/mozilla-central/rev/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/testing/web-platform/tests/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html Which isn't using the feature at all and I'm not sure why is it supposed to pass (why prefers-color-scheme: dark is supposed to match that SVG image). This test fails in all browsers apparently: https://wpt.fyi/results/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html?label=master&label=experimental&aligned I sent https://github.com/web-platform-tests/wpt/pull/31407 to remove it and hopefully get it reviewed by some Chromium folks. Differential Revision: https://phabricator.services.mozilla.com/D129746 --- components/style/gecko/media_queries.rs | 17 ++++++++++++----- components/style/properties/cascade.rs | 4 ++-- .../properties/longhands/inherited_ui.mako.rs | 1 + 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 6e82404de1d..7bab0b002d1 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -93,7 +93,9 @@ impl Device { document, default_values: ComputedValues::default_values(doc), root_font_size: AtomicU32::new(FONT_MEDIUM_PX.to_bits()), - body_text_color: AtomicUsize::new(prefs.mColors.mDefault as usize), + // This gets updated when we see the , so it doesn't really + // matter which color-scheme we look at here. + body_text_color: AtomicUsize::new(prefs.mLightColors.mDefault as usize), used_root_font_size: AtomicBool::new(false), used_font_metrics: AtomicBool::new(false), used_viewport_size: AtomicBool::new(false), @@ -386,13 +388,18 @@ impl Device { } /// Returns the default background color. - pub fn default_background_color(&self) -> RGBA { - convert_nscolor_to_rgba(self.pref_sheet_prefs().mColors.mDefaultBackground) + /// + /// This is only for forced-colors/high-contrast, so looking at light colors + /// is ok. + pub fn default_background_color_for_forced_colors(&self) -> RGBA { + convert_nscolor_to_rgba(self.pref_sheet_prefs().mLightColors.mDefaultBackground) } /// Returns the default foreground color. - pub fn default_color(&self) -> RGBA { - convert_nscolor_to_rgba(self.pref_sheet_prefs().mColors.mDefault) + /// + /// See above for looking at light colors only. + pub fn default_color_for_forced_colors(&self) -> RGBA { + convert_nscolor_to_rgba(self.pref_sheet_prefs().mLightColors.mDefault) } /// Returns the current effective text zoom. diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 0834da93e89..fe5bae33fe8 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -430,7 +430,7 @@ fn tweak_when_ignoring_colors( // widget background color's rgb channels but not alpha... let alpha = alpha_channel(color, context); if alpha != 0 { - let mut color = context.builder.device.default_background_color(); + let mut color = context.builder.device.default_background_color_for_forced_colors(); color.alpha = alpha; declarations_to_apply_unless_overriden .push(PropertyDeclaration::BackgroundColor(color.into())) @@ -448,7 +448,7 @@ fn tweak_when_ignoring_colors( // override this with a non-transparent color, then override it with // the default color. Otherwise just let it inherit through. if context.builder.get_parent_inherited_text().clone_color().alpha == 0 { - let color = context.builder.device.default_color(); + let color = context.builder.device.default_color_for_forced_colors(); declarations_to_apply_unless_overriden.push(PropertyDeclaration::Color( specified::ColorPropertyValue(color.into()), )) diff --git a/components/style/properties/longhands/inherited_ui.mako.rs b/components/style/properties/longhands/inherited_ui.mako.rs index 4fb702a97b4..44dfdd74a2a 100644 --- a/components/style/properties/longhands/inherited_ui.mako.rs +++ b/components/style/properties/longhands/inherited_ui.mako.rs @@ -104,6 +104,7 @@ ${helpers.predefined_type( gecko_pref="layout.css.color-scheme.enabled", animation_value_type="discrete", has_effect_on_gecko_scrollbars=False, + ignored_when_colors_disabled=True, enabled_in="chrome", )}