From 1f564dfb236be45b5892d0fac37b94f5a7ab66bc Mon Sep 17 00:00:00 2001 From: Steph Toyofuku Date: Mon, 11 Mar 2024 15:09:20 -0700 Subject: [PATCH] Fix color for inline styles with background color in night mode Use CSS to target parsed content where a background color is defined inline without an accompanying text color. Additionally, add an ADR documenting this decision to the repo Visual change in night mode only, as the text color will now be correct Bug: T358797 Change-Id: If375c4c9691462d314e91a74da0fc8365137cd8c --- ...t-colors-in-inline-styles-in-night-mode.md | 39 +++++++++++++++++++ .../content/hacks.less | 10 +++++ 2 files changed, 49 insertions(+) create mode 100644 adr/0005-autocorrect-colors-in-inline-styles-in-night-mode.md diff --git a/adr/0005-autocorrect-colors-in-inline-styles-in-night-mode.md b/adr/0005-autocorrect-colors-in-inline-styles-in-night-mode.md new file mode 100644 index 000000000..43f6c17dc --- /dev/null +++ b/adr/0005-autocorrect-colors-in-inline-styles-in-night-mode.md @@ -0,0 +1,39 @@ +# 5. Adopt ADRs (Architectural decision records) + +Date: 2024-03-11 + +## Status + +Accepted + +## Context + +As the team works on developing night mode, the largest hurdle remains user- +generated content. One particular class of user-generated content that we've +run up against is the use of inline styles to define a background color, with +the assumption the text color would remain constant. With the introduction of +night mode, this assumption is broken, and we are running into color contrast +issues for light-colored text on a background that was presumed to be +contrasting with a darker text color + +## Decision + +After evaluating the expected performance impacts, we have decided that a CSS +approach targeting `[style*=background]` that sets the color explicitly to +`#333` would be sufficient to address this concern. While we considered a php- +based parser approach, it was found that the CSS approach was performant enough +to meet our needs and considerably more straightforward to implement + +See https://phabricator.wikimedia.org/T358240#9591458 for further details + +## Consequences + +The most direct consequence of this change is that we will be modifying the +display of the page from what the authors explicitly intended for it to be. +We're quite confident that in the vast majority of cases this will not be an +issue, as the omission of font color was most likely predicated upon the +assumption it would remain the same in all cases, but we understand and +acknowledge that this could lead to issues where a user is unhappy with the +color being modified. To mitigate this issue, we have deliberately written the +style such that a defined color will take precendence over this, so ideally the +risk of this is low diff --git a/resources/skins.minerva.base.styles/content/hacks.less b/resources/skins.minerva.base.styles/content/hacks.less index fdab674b8..806a39b4c 100644 --- a/resources/skins.minerva.base.styles/content/hacks.less +++ b/resources/skins.minerva.base.styles/content/hacks.less @@ -229,6 +229,11 @@ html.skin-night-mode-clientpref-1 { .night-mode-strip-all-colors-when-safe(); } } + + // T358797 - if a background color is specified, assume they wanted the day mode font color + .mw-parser-output [ style*='background' ] { + color: #333; // not !important so that if a color is also specified it will take priority + } } @media ( prefers-color-scheme: dark ) { @@ -268,6 +273,11 @@ html.skin-night-mode-clientpref-1 { color: var( --color-base ) !important; background-color: var( --background-color-interactive-subtle ) !important; } + + // T358797 - if a background color is specified, assume they wanted the day mode font color + .mw-parser-output [ style*='background' ] { + color: #333; // not !important so that if a color is also specified it will take priority + } } } // stylelint-enable no-descending-specificity