fix(theming): Adjust theming util to calculate primary element color based on WCAG color contrast

apps/theming/css/default.css

@@ -77,9 +77,9 @@
   --color-primary-light-text: #00293f;
   --color-primary-light-hover: #dbe4ea;
   --color-primary-element: #00679e;
-  --color-primary-element-hover: #1674a6;
+  --color-primary-element-hover: #005a8a;
   --color-primary-element-text: #ffffff;
-  --color-primary-element-text-dark: #f0f0f0;
+  --color-primary-element-text-dark: #f5f5f5;
   --color-primary-element-light: #e5eff5;
   --color-primary-element-light-hover: #dbe4ea;
   --color-primary-element-light-text: #00293f;

apps/theming/lib/Themes/CommonThemeTrait.php

@@ -40,9 +40,10 @@ trait CommonThemeTrait {
 	 */
 	protected function generatePrimaryVariables(string $colorMainBackground, string $colorMainText): array {
 		$isBrightColor = $this->util->isBrightColor($colorMainBackground);
-		$colorPrimaryElement = $this->util->elementColor($this->primaryColor, $isBrightColor);
+		$colorPrimaryElement = $this->util->elementColor($this->primaryColor, $isBrightColor, $colorMainBackground);
 		$colorPrimaryLight = $this->util->mix($colorPrimaryElement, $colorMainBackground, -80);
 		$colorPrimaryElementLight = $this->util->mix($colorPrimaryElement, $colorMainBackground, -80);
+		$invertPrimaryTextColor = $this->util->invertTextColor($colorPrimaryElement);
 
 		// primary related colours
 		return [
@@ -66,10 +67,10 @@ protected function generatePrimaryVariables(string $colorMainBackground, string
 
 			// used for buttons, inputs...
 			'--color-primary-element' => $colorPrimaryElement,
-			'--color-primary-element-hover' => $this->util->mix($colorPrimaryElement, $colorMainBackground, 82),
-			'--color-primary-element-text' => $this->util->invertTextColor($colorPrimaryElement) ? '#000000' : '#ffffff',
+			'--color-primary-element-hover' => $invertPrimaryTextColor ? $this->util->lighten($colorPrimaryElement, 4) : $this->util->darken($colorPrimaryElement, 4),
+			'--color-primary-element-text' => $invertPrimaryTextColor ? '#000000' : '#ffffff',
 			// mostly used for disabled states
-			'--color-primary-element-text-dark' => $this->util->darken($this->util->invertTextColor($colorPrimaryElement) ? '#000000' : '#ffffff', 6),
+			'--color-primary-element-text-dark' => $invertPrimaryTextColor ? $this->util->lighten('#000000', 4) : $this->util->darken('#ffffff', 4),
 
 			// used for hover/focus states
 			'--color-primary-element-light' => $colorPrimaryElementLight,

apps/theming/lib/Util.php

@@ -57,7 +57,7 @@ public function __construct(IConfig $config, IAppManager $appManager, IAppData $
 	 * @return bool
 	 */
 	public function invertTextColor(string $color): bool {
-		return $this->isBrightColor($color);
+		return $this->colorContrast($color, '#ffffff') < 4.5;
 	}
 
 	/**
@@ -81,7 +81,28 @@ public function isBrightColor(string $color): bool {
 	 * @param ?bool $brightBackground
 	 * @return string
 	 */
-	public function elementColor($color, ?bool $brightBackground = null) {
+	public function elementColor($color, ?bool $brightBackground = null, ?string $backgroundColor = null) {
+		if ($backgroundColor !== null) {
+			$brightBackground = $brightBackground ?? $this->isBrightColor($backgroundColor);
+			// Minimal amount that is possible to change the luminance
+			$epsilon = 1.0 / 255.0;
+			// Current iteration to prevent infinite loops
+			$iteration = 0;
+			// We need to keep blurred backgrounds in mind which might be mixed with the background
+			$blurredBackground = $this->mix($backgroundColor, $brightBackground ? $color : '#ffffff', 66);
+			$contrast = $this->colorContrast($color, $blurredBackground);
+
+			// Min. element contrast is 3:1 but we need to keep hover states in mind -> min 3.2:1
+			while ($contrast < 3.2 && $iteration++ < 100) {
+				$hsl = Color::hexToHsl($color);
+				$hsl['L'] = max(0, min(1, $hsl['L'] + ($brightBackground ? -$epsilon : $epsilon)));
+				$color = '#' . Color::hslToHex($hsl);
+				$contrast = $this->colorContrast($color, $blurredBackground);
+			}
+			return $color;
+		}
+
+		// Fallback for legacy calling
 		$luminance = $this->calculateLuminance($color);
 
 		if ($brightBackground !== false && $luminance > 0.8) {
@@ -139,12 +160,38 @@ public function calculateLuminance(string $color): float {
 	}
 
 	/**
+	 * Calculate the Luma according to WCAG 2
+	 * http://www.w3.org/TR/WCAG20/#relativeluminancedef
 	 * @param string $color rgb color value
 	 * @return float
 	 */
 	public function calculateLuma(string $color): float {
-		[$red, $green, $blue] = $this->hexToRGB($color);
-		return (0.2126 * $red + 0.7152 * $green + 0.0722 * $blue) / 255;
+		$rgb = $this->hexToRGB($color);
+
+		// Normalize the values by converting to float and applying the rules from WCAG2.0
+		$rgb = array_map(function (int $color) {
+			$color = $color / 255.0;
+			if ($color <= 0.03928) {
+				return $color / 12.92;
+			} else {
+				return pow((($color + 0.055) / 1.055), 2.4);
+			}
+		}, $rgb);
+
+		[$red, $green, $blue] = $rgb;
+		return (0.2126 * $red + 0.7152 * $green + 0.0722 * $blue);
+	}
+
+	/**
+	 * Calculat the color contrast according to WCAG 2
+	 * http://www.w3.org/TR/WCAG20/#contrast-ratiodef
+	 * @param string $color1 The first color
+	 * @param string $color2 The second color
+	 */
+	public function colorContrast(string $color1, string $color2): float {
+		$luminance1 = $this->calculateLuma($color1) + 0.05;
+		$luminance2 = $this->calculateLuma($color2) + 0.05;
+		return max($luminance1, $luminance2) / min($luminance1, $luminance2);
 	}
 
 	/**

apps/theming/tests/Themes/AccessibleThemeTestCase.php

@@ -0,0 +1,122 @@
+<?php
+/**
+ * @copyright Copyright (c) 2023 Ferdinand Thiessen <opensource@fthiessen.de>
+ *
+ * @author Ferdinand Thiessen <opensource@fthiessen.de>
+ *
+ * @license AGPL-3.0-or-later
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OCA\Theming\Tests\Themes;
+
+use OCA\Theming\ITheme;
+use OCA\Theming\Util;
+use Test\TestCase;
+
+class AccessibleThemeTestCase extends TestCase {
+	protected ITheme $theme;
+	protected Util $util;
+
+	public function dataAccessibilityPairs() {
+		return [
+			'primary-element on background' => [
+				[
+					'--color-primary-element',
+					'--color-primary-element-hover',
+				],
+				[
+					'--color-main-background',
+					'--color-background-hover',
+					'--color-background-dark',
+					'--color-background-darker',
+					'--color-main-background-blur',
+				],
+				3.0,
+			],
+			'primary-element-text' => [
+				[
+					'--color-primary-element-text',
+					'--color-primary-element-text-dark',
+				],
+				[
+					'--color-primary-element',
+					'--color-primary-element-hover',
+				],
+				4.5,
+			],
+			'primary-element-light-text' => [
+				['--color-primary-element-light-text'],
+				[
+					'--color-primary-element-light',
+					'--color-primary-element-light-hover',
+				],
+				4.5,
+			],
+			'main-text' => [
+				['--color-main-text'],
+				[
+					'--color-main-background',
+					'--color-background-hover',
+					'--color-background-dark',
+					'--color-background-darker',
+					'--color-main-background-blur',
+				],
+				4.5,
+			],
+			'max-contrast-text' => [
+				['--color-text-maxcontrast'],
+				[
+					'--color-main-background',
+					'--color-background-hover',
+					'--color-background-dark',
+				],
+				4.5,
+			],
+			'max-contrast text-on blur' => [
+				['--color-text-maxcontrast-background-blur'],
+				[
+					'--color-main-background-blur',
+				],
+				4.5,
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider dataAccessibilityPairs
+	 */
+	public function testAccessibilityOfVariables($mainColors, $backgroundColors, $minContrast) {
+		if (!isset($this->theme)) {
+			$this->markTestSkipped('You need to setup $this->theme in your setUp function');
+		} elseif (!isset($this->util)) {
+			$this->markTestSkipped('You need to setup $this->util in your setUp function');
+		}
+
+		$variables = $this->theme->getCSSVariables();
+
+		// Blur effect does not work so we mockup the color - worst supported case is the default "clouds" background image (on dark themes the clouds with white color are bad on bright themes the primary color as sky is bad)
+		$variables['--color-main-background-blur'] = $this->util->mix($variables['--color-main-background'], $this->util->isBrightColor($variables['--color-main-background']) ? $variables['--color-primary'] : '#ffffff', 75);
+
+		foreach ($backgroundColors as $background) {
+			$this->assertStringStartsWith('#', $variables[$background], 'Is not a plain color variable - consider to remove or fix this test');
+			foreach ($mainColors as $main) {
+				$this->assertStringStartsWith('#', $variables[$main], 'Is not a plain color variable - consider to remove or fix this test');
+				$realContrast = $this->util->colorContrast($variables[$main], $variables[$background]);
+				$this->assertGreaterThanOrEqual($minContrast, $realContrast, "Contrast is not high enough for $main (" . $variables[$main] . ") on $background (" . $variables[$background] . ')');
+			}
+		}
+	}
+}

apps/theming/tests/Themes/DefaultThemeTest.php

@@ -20,7 +20,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
-namespace OCA\Theming\Tests\Service;
+namespace OCA\Theming\Tests\Themes;
 
 use OCA\Theming\AppInfo\Application;
 use OCA\Theming\ImageManager;
@@ -36,9 +36,8 @@
 use OCP\IURLGenerator;
 use OCP\IUserSession;
 use PHPUnit\Framework\MockObject\MockObject;
-use Test\TestCase;
 
-class DefaultThemeTest extends TestCase {
+class DefaultThemeTest extends AccessibleThemeTestCase {
 	/** @var ThemingDefaults|MockObject */
 	private $themingDefaults;
 	/** @var IUserSession|MockObject */
@@ -54,8 +53,6 @@ class DefaultThemeTest extends TestCase {
 	/** @var IAppManager|MockObject */
 	private $appManager;
 
-	private DefaultTheme $defaultTheme;
-
 	protected function setUp(): void {
 		$this->themingDefaults = $this->createMock(ThemingDefaults::class);
 		$this->userSession = $this->createMock(IUserSession::class);
@@ -65,7 +62,7 @@ protected function setUp(): void {
 		$this->l10n = $this->createMock(IL10N::class);
 		$this->appManager = $this->createMock(IAppManager::class);
 
-		$util = new Util(
+		$this->util = new Util(
 			$this->config,
 			$this->appManager,
 			$this->createMock(IAppData::class),
@@ -101,8 +98,8 @@ protected function setUp(): void {
 				return "/$app/img/$filename";
 			});
 
-		$this->defaultTheme = new DefaultTheme(
-			$util,
+		$this->theme = new DefaultTheme(
+			$this->util,
 			$this->themingDefaults,
 			$this->userSession,
 			$this->urlGenerator,
@@ -117,31 +114,31 @@ protected function setUp(): void {
 
 
 	public function testGetId() {
-		$this->assertEquals('default', $this->defaultTheme->getId());
+		$this->assertEquals('default', $this->theme->getId());
 	}
 
 	public function testGetType() {
-		$this->assertEquals(ITheme::TYPE_THEME, $this->defaultTheme->getType());
+		$this->assertEquals(ITheme::TYPE_THEME, $this->theme->getType());
 	}
 
 	public function testGetTitle() {
-		$this->assertEquals('System default theme', $this->defaultTheme->getTitle());
+		$this->assertEquals('System default theme', $this->theme->getTitle());
 	}
 
 	public function testGetEnableLabel() {
-		$this->assertEquals('Enable the system default', $this->defaultTheme->getEnableLabel());
+		$this->assertEquals('Enable the system default', $this->theme->getEnableLabel());
 	}
 
 	public function testGetDescription() {
-		$this->assertEquals('Using the default system appearance.', $this->defaultTheme->getDescription());
+		$this->assertEquals('Using the default system appearance.', $this->theme->getDescription());
 	}
 
 	public function testGetMediaQuery() {
-		$this->assertEquals('', $this->defaultTheme->getMediaQuery());
+		$this->assertEquals('', $this->theme->getMediaQuery());
 	}
 
 	public function testGetCustomCss() {
-		$this->assertEquals('', $this->defaultTheme->getCustomCss());
+		$this->assertEquals('', $this->theme->getCustomCss());
 	}
 
 	/**
@@ -151,7 +148,7 @@ public function testGetCustomCss() {
 	public function testThemindDisabledFallbackCss() {
 		// Generate variables
 		$variables = '';
-		foreach ($this->defaultTheme->getCSSVariables() as $variable => $value) {
+		foreach ($this->theme->getCSSVariables() as $variable => $value) {
 			$variables .= "  $variable: $value;" . PHP_EOL;
 		};
 

apps/theming/tests/UtilTest.php

@@ -35,19 +35,20 @@
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\IConfig;
+use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class UtilTest extends TestCase {
 
 	/** @var Util */
 	protected $util;
-	/** @var IConfig */
+	/** @var IConfig|MockObject */
 	protected $config;
-	/** @var IAppData */
+	/** @var IAppData|MockObject */
 	protected $appData;
-	/** @var IAppManager */
+	/** @var IAppManager|MockObject */
 	protected $appManager;
-	/** @var ImageManager */
+	/** @var ImageManager|MockObject */
 	protected $imageManager;
 
 	protected function setUp(): void {
@@ -59,11 +60,29 @@ protected function setUp(): void {
 		$this->util = new Util($this->config, $this->appManager, $this->appData, $this->imageManager);
 	}
 
+	public function dataColorContrast() {
+		return [
+			['#ffffff', '#FFFFFF', 1],
+			['#000000', '#000000', 1],
+			['#ffffff', '#000000', 21],
+			['#000000', '#FFFFFF', 21],
+			['#9E9E9E', '#353535', 4.578],
+			['#353535', '#9E9E9E', 4.578],
+		];
+	}
+
+	/**
+	 * @dataProvider dataColorContrast
+	 */
+	public function testColorContrast(string $color1, string $color2, $contrast) {
+		$this->assertEqualsWithDelta($contrast, $this->util->colorContrast($color1, $color2), .001);
+	}
+
 	public function dataInvertTextColor() {
 		return [
 			['#ffffff', true],
 			['#000000', false],
-			['#0082C9', false],
+			['#00679e', false],
 			['#ffff00', true],
 		];
 	}