From b7ca7d01fbe222e9782fa2940d683c60959f59a1 Mon Sep 17 00:00:00 2001 From: Jack Arru Date: Tue, 9 Jun 2026 10:54:02 +0200 Subject: [PATCH 1/2] feat(theming): allow request-scoped light/dark theme override Support ?theme=light and ?theme=dark as a request-scoped override that is not persisted to user settings and never bypasses an admin-configured enforce_theme. Invalid, empty, non-string or unknown values are ignored, and non-visual themes such as the OpenDyslexic font theme are preserved. The override is applied only in ThemesService::getEnabledThemes() so that getThemes() and theme registration remain unchanged. Assisted-by: Cline Signed-off-by: Jack Arru --- apps/theming/lib/Service/ThemesService.php | 55 ++++++++++- .../tests/Service/ThemesServiceTest.php | 91 +++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/apps/theming/lib/Service/ThemesService.php b/apps/theming/lib/Service/ThemesService.php index 3380b33d0c957..1818b22bfbc44 100644 --- a/apps/theming/lib/Service/ThemesService.php +++ b/apps/theming/lib/Service/ThemesService.php @@ -16,17 +16,25 @@ use OCA\Theming\Themes\HighContrastTheme; use OCA\Theming\Themes\LightTheme; use OCP\IConfig; +use OCP\IRequest; use OCP\IUser; use OCP\IUserSession; use Psr\Log\LoggerInterface; class ThemesService { + private const REQUEST_THEME_PARAM = 'theme'; + private const REQUEST_THEME_OVERRIDES = [ + 'light', + 'dark', + ]; + /** @var ITheme[] */ private array $themesProviders; public function __construct( private IUserSession $userSession, private IConfig $config, + private IRequest $request, private LoggerInterface $logger, private DefaultTheme $defaultTheme, LightTheme $lightTheme, @@ -161,6 +169,12 @@ public function getEnabledThemes() { if ($enforcedTheme !== '') { return [$enforcedTheme]; } + + $requestThemeOverride = $this->getRequestThemeOverride(); + if ($requestThemeOverride !== null) { + return [$requestThemeOverride]; + } + return []; } @@ -171,12 +185,51 @@ public function getEnabledThemes() { } try { - return $enabledThemes; + return $this->applyRequestThemeOverride($enabledThemes); } catch (\Exception $e) { return []; } } + /** + * Apply a request-scoped light/dark theme override without persisting it. + * + * @param list $themes + * @return list + */ + private function applyRequestThemeOverride(array $themes): array { + $requestThemeOverride = $this->getRequestThemeOverride(); + if ($requestThemeOverride === null) { + return $themes; + } + + $theme = $this->themesProviders[$requestThemeOverride]; + $themes = array_filter($themes, function (string $themeId) use ($theme): bool { + return !isset($this->themesProviders[$themeId]) + || $this->themesProviders[$themeId]->getType() !== $theme->getType(); + }); + + return array_values(array_unique(array_merge($themes, [$requestThemeOverride]))); + } + + private function getRequestThemeOverride(): ?string { + $requestThemeOverride = $this->request->getParam(self::REQUEST_THEME_PARAM, ''); + if (!is_string($requestThemeOverride)) { + return null; + } + + $requestThemeOverride = strtolower(trim($requestThemeOverride)); + if (!in_array($requestThemeOverride, self::REQUEST_THEME_OVERRIDES, true)) { + return null; + } + + if (!isset($this->themesProviders[$requestThemeOverride])) { + return null; + } + + return $requestThemeOverride; + } + /** * Set the list of enabled themes * for the logged-in user diff --git a/apps/theming/tests/Service/ThemesServiceTest.php b/apps/theming/tests/Service/ThemesServiceTest.php index 9d3dc9c3e8937..b5103112fadff 100644 --- a/apps/theming/tests/Service/ThemesServiceTest.php +++ b/apps/theming/tests/Service/ThemesServiceTest.php @@ -23,6 +23,7 @@ use OCP\App\IAppManager; use OCP\IConfig; use OCP\IL10N; +use OCP\IRequest; use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserSession; @@ -33,6 +34,7 @@ class ThemesServiceTest extends TestCase { private IUserSession&MockObject $userSession; private IConfig&MockObject $config; + private IRequest&MockObject $request; private LoggerInterface&MockObject $logger; private ThemingDefaults&MockObject $themingDefaults; @@ -44,6 +46,7 @@ class ThemesServiceTest extends TestCase { protected function setUp(): void { $this->userSession = $this->createMock(IUserSession::class); $this->config = $this->createMock(IConfig::class); + $this->request = $this->createMock(IRequest::class); $this->logger = $this->createMock(LoggerInterface::class); $this->themingDefaults = $this->createMock(ThemingDefaults::class); @@ -60,6 +63,7 @@ protected function setUp(): void { $this->themesService = new ThemesService( $this->userSession, $this->config, + $this->request, $this->logger, ...array_values($this->themes) ); @@ -232,6 +236,93 @@ public function testGetEnabledThemes(): void { $this->assertEquals(['default'], $this->themesService->getEnabledThemes()); } + public function testGetEnabledThemesRequestThemeForGuest(): void { + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn(null); + $this->config->expects($this->once()) + ->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn(''); + $this->request->expects($this->once()) + ->method('getParam') + ->with('theme', '') + ->willReturn('dark'); + + $this->assertEquals(['dark'], $this->themesService->getEnabledThemes()); + } + + public function testGetEnabledThemesRequestThemeOverridesUserTheme(): void { + $user = $this->createMock(IUser::class); + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn($user); + $user->expects($this->any()) + ->method('getUID') + ->willReturn('user'); + + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('user', Application::APP_ID, 'enabled-themes', '["default"]') + ->willReturn(json_encode(['dark', 'opendyslexic'])); + $this->config->expects($this->once()) + ->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn(''); + $this->request->expects($this->once()) + ->method('getParam') + ->with('theme', '') + ->willReturn('light'); + + $this->assertEquals(['opendyslexic', 'light'], $this->themesService->getEnabledThemes()); + } + + public function testGetEnabledThemesInvalidRequestTheme(): void { + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn(null); + $this->config->expects($this->once()) + ->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn(''); + $this->request->expects($this->once()) + ->method('getParam') + ->with('theme', '') + ->willReturn('sepia'); + + $this->assertEquals([], $this->themesService->getEnabledThemes()); + } + + public function testGetEnabledThemesNonStringRequestThemeIsIgnored(): void { + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn(null); + $this->config->expects($this->once()) + ->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn(''); + $this->request->expects($this->once()) + ->method('getParam') + ->with('theme', '') + ->willReturn(['dark']); + + $this->assertEquals([], $this->themesService->getEnabledThemes()); + } + + public function testGetEnabledThemesRequestThemeDoesNotOverrideEnforcedTheme(): void { + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn(null); + $this->config->expects($this->once()) + ->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn('light'); + $this->request->expects($this->never()) + ->method('getParam'); + + $this->assertEquals(['light'], $this->themesService->getEnabledThemes()); + } + public function testGetEnabledThemesEnforced(): void { $user = $this->createMock(IUser::class); $this->userSession->expects($this->any()) From eea494b397ee9e124bd8ed3d5fd602d6d7eef842 Mon Sep 17 00:00:00 2001 From: Jack Arru Date: Tue, 9 Jun 2026 14:17:27 +0200 Subject: [PATCH 2/2] fix(theming): force request theme override over prefers-color-scheme The request-scoped ?theme=light / ?theme=dark override set the data-theme-* body attribute but ThemeInjectionService still injected the OS prefers-color-scheme stylesheets on :root and a combined color-scheme meta. On a dark-OS machine the root element and native controls stayed dark, so the override appeared not to work. When an override is active (and no enforce_theme is set), force the chosen theme unconditionally on :root, drop the prefers-color-scheme auto-switching stylesheets, and expose only the override color-scheme meta. Behavior without an override and admin enforce_theme precedence are unchanged. Assisted-by: Cline Signed-off-by: Jack Arru --- .../lib/Service/ThemeInjectionService.php | 45 +++- apps/theming/lib/Service/ThemesService.php | 13 +- .../Service/ThemeInjectionServiceTest.php | 224 ++++++++++++++++++ 3 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 apps/theming/tests/Service/ThemeInjectionServiceTest.php diff --git a/apps/theming/lib/Service/ThemeInjectionService.php b/apps/theming/lib/Service/ThemeInjectionService.php index b1c975baf0736..2b8870ff79a03 100644 --- a/apps/theming/lib/Service/ThemeInjectionService.php +++ b/apps/theming/lib/Service/ThemeInjectionService.php @@ -36,6 +36,19 @@ public function __construct( public function injectHeaders(): void { $themes = $this->themesService->getThemes(); $defaultTheme = $themes[$this->defaultTheme->getId()]; + + // A request-scoped light/dark override must win over the OS + // `prefers-color-scheme` preference, so we force it on `:root` + // instead of relying on the media-query auto-switching. + // An admin-enforced theme always takes precedence over the override. + $requestThemeOverride = $this->config->getSystemValueString('enforce_theme', '') === '' + ? $this->themesService->getRequestThemeOverride() + : null; + if ($requestThemeOverride !== null && isset($themes[$requestThemeOverride])) { + $this->injectOverrideHeaders($themes, $defaultTheme, $themes[$requestThemeOverride]); + return; + } + $mediaThemes = array_filter($themes, function ($theme) { // Check if the theme provides a media query return (bool)$theme->getMediaQuery(); @@ -62,6 +75,36 @@ public function injectHeaders(): void { $this->addThemeMetaHeaders($themes); } + /** + * Inject the headers for a request-scoped light/dark theme override. + * + * The override has to take precedence over the OS `prefers-color-scheme` + * preference, so the overridden theme is forced on `:root` (without any + * media query) and only its `color-scheme` meta is exposed. + * + * @param ITheme[] $themes all registered themes + * @param ITheme $defaultTheme the default theme used as a fallback + * @param ITheme $overrideTheme the theme requested through the query string + */ + private function injectOverrideHeaders(array $themes, ITheme $defaultTheme, ITheme $overrideTheme): void { + // Default theme fallback + $this->addThemeHeaders($defaultTheme); + + // Force the overridden theme unconditionally on `:root` + $this->addThemeHeaders($overrideTheme, true); + + // Keep body-scoped themes so `[data-theme-*]` selectors keep working + foreach ($themes as $theme) { + if ($theme->getId() === $this->defaultTheme->getId()) { + continue; + } + $this->addThemeHeaders($theme, false); + } + + // Only expose the overridden theme color-scheme meta + $this->addThemeMetaHeaders([$overrideTheme->getId() => $overrideTheme]); + } + /** * Inject theme header into rendered page * @@ -92,7 +135,7 @@ private function addThemeMetaHeaders(array $themes): void { $metaHeaders = []; // Meta headers - foreach ($this->themesService->getThemes() as $theme) { + foreach ($themes as $theme) { if (!empty($theme->getMeta())) { foreach ($theme->getMeta() as $meta) { if (!isset($meta['name']) || !isset($meta['content'])) { diff --git a/apps/theming/lib/Service/ThemesService.php b/apps/theming/lib/Service/ThemesService.php index 1818b22bfbc44..b9d082afe3622 100644 --- a/apps/theming/lib/Service/ThemesService.php +++ b/apps/theming/lib/Service/ThemesService.php @@ -212,7 +212,18 @@ private function applyRequestThemeOverride(array $themes): array { return array_values(array_unique(array_merge($themes, [$requestThemeOverride]))); } - private function getRequestThemeOverride(): ?string { + /** + * Get the request-scoped light/dark theme override, if any. + * + * Returns the id of a registered light/dark theme requested through the + * `theme` query parameter, or null when no valid override is present. + * + * Note: callers are responsible for honoring an admin-enforced theme, + * which always takes precedence over this request-scoped override. + * + * @return ?string + */ + public function getRequestThemeOverride(): ?string { $requestThemeOverride = $this->request->getParam(self::REQUEST_THEME_PARAM, ''); if (!is_string($requestThemeOverride)) { return null; diff --git a/apps/theming/tests/Service/ThemeInjectionServiceTest.php b/apps/theming/tests/Service/ThemeInjectionServiceTest.php new file mode 100644 index 0000000000000..2535590077765 --- /dev/null +++ b/apps/theming/tests/Service/ThemeInjectionServiceTest.php @@ -0,0 +1,224 @@ +urlGenerator = $this->createMock(IURLGenerator::class); + $this->themesService = $this->createMock(ThemesService::class); + $this->util = $this->createMock(Util::class); + $this->config = $this->createMock(IConfig::class); + $this->userSession = $this->createMock(IUserSession::class); + $this->defaultTheme = $this->createMock(DefaultTheme::class); + + $this->defaultTheme->method('getId')->willReturn('default'); + + $this->urlGenerator->method('linkToRoute') + ->willReturnCallback(function (string $route, array $params): string { + return '/css/' . $params['themeId'] . '?plain=' . ($params['plain'] ? '1' : '0'); + }); + + $this->service = new ThemeInjectionService( + $this->urlGenerator, + $this->themesService, + $this->defaultTheme, + $this->util, + $this->config, + $this->userSession, + ); + + // Reset the static headers collected by the service + \OC_Util::$headers = []; + } + + protected function tearDown(): void { + \OC_Util::$headers = []; + parent::tearDown(); + } + + /** + * @return ITheme[] + */ + private function buildThemes(): array { + $default = $this->createMock(DefaultTheme::class); + $default->method('getId')->willReturn('default'); + $default->method('getMediaQuery')->willReturn(''); + $default->method('getMeta')->willReturn([]); + + $light = $this->createMock(LightTheme::class); + $light->method('getId')->willReturn('light'); + $light->method('getMediaQuery')->willReturn('(prefers-color-scheme: light)'); + $light->method('getMeta')->willReturn([['name' => 'color-scheme', 'content' => 'light']]); + + $dark = $this->createMock(DarkTheme::class); + $dark->method('getId')->willReturn('dark'); + $dark->method('getMediaQuery')->willReturn('(prefers-color-scheme: dark)'); + $dark->method('getMeta')->willReturn([['name' => 'color-scheme', 'content' => 'dark']]); + + $lightHc = $this->createMock(HighContrastTheme::class); + $lightHc->method('getId')->willReturn('light-highcontrast'); + $lightHc->method('getMediaQuery')->willReturn('(prefers-contrast: more)'); + $lightHc->method('getMeta')->willReturn([]); + + $darkHc = $this->createMock(DarkHighContrastTheme::class); + $darkHc->method('getId')->willReturn('dark-highcontrast'); + $darkHc->method('getMediaQuery')->willReturn('(prefers-color-scheme: dark) and (prefers-contrast: more)'); + $darkHc->method('getMeta')->willReturn([]); + + $dyslexic = $this->createMock(DyslexiaFont::class); + $dyslexic->method('getId')->willReturn('opendyslexic'); + $dyslexic->method('getMediaQuery')->willReturn(''); + $dyslexic->method('getMeta')->willReturn([]); + + return [ + 'default' => $default, + 'light' => $light, + 'dark' => $dark, + 'light-highcontrast' => $lightHc, + 'dark-highcontrast' => $darkHc, + 'opendyslexic' => $dyslexic, + ]; + } + + /** + * @return array{links: list, metas: list} + */ + private function collectHeaders(): array { + $links = []; + $metas = []; + foreach (\OC_Util::$headers as $header) { + $attrs = $header['attributes']; + if ($header['tag'] === 'link' && ($attrs['class'] ?? '') === 'theme') { + preg_match('#/css/([a-z-]+)\?plain=([01])#', $attrs['href'], $m); + $links[] = [ + 'themeId' => $m[1], + 'media' => $attrs['media'] ?? null, + 'plain' => $m[2] === '1', + ]; + } elseif ($header['tag'] === 'meta' && ($attrs['name'] ?? '') === 'color-scheme') { + $metas[] = ['name' => $attrs['name'], 'content' => $attrs['content']]; + } + } + return ['links' => $links, 'metas' => $metas]; + } + + public function testInjectHeadersWithoutOverrideUsesMediaQueries(): void { + $themes = $this->buildThemes(); + $this->themesService->method('getThemes')->willReturn($themes); + $this->config->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn(''); + $this->themesService->method('getRequestThemeOverride')->willReturn(null); + + $this->service->injectHeaders(); + $collected = $this->collectHeaders(); + + // Media-query based stylesheets must be present for auto-switching + $mediaLinks = array_filter($collected['links'], fn ($l) => $l['media'] === '(prefers-color-scheme: dark)' && $l['plain']); + $this->assertCount(1, $mediaLinks, 'Dark prefers-color-scheme stylesheet should be injected when no override'); + + // Color scheme meta should contain both light and dark + $this->assertCount(1, $collected['metas']); + $this->assertEqualsCanonicalizing(['light', 'dark'], explode(' ', $collected['metas'][0]['content'])); + } + + public function testInjectHeadersWithLightOverrideForcesRootWithoutMedia(): void { + $themes = $this->buildThemes(); + $this->themesService->method('getThemes')->willReturn($themes); + $this->config->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn(''); + $this->themesService->method('getRequestThemeOverride')->willReturn('light'); + + $this->service->injectHeaders(); + $collected = $this->collectHeaders(); + + // No prefers-color-scheme media stylesheet must be injected + foreach ($collected['links'] as $link) { + $this->assertStringNotContainsString('prefers-color-scheme', (string)$link['media']); + } + + // The light theme must be forced on :root (plain, no media) + $forcedLight = array_filter( + $collected['links'], + fn ($l) => $l['themeId'] === 'light' && $l['plain'] && ($l['media'] === null || $l['media'] === ''), + ); + $this->assertCount(1, $forcedLight, 'Light theme must be forced on :root without a media query'); + + // Only the light color-scheme meta must be exposed + $this->assertCount(1, $collected['metas']); + $this->assertSame('light', $collected['metas'][0]['content']); + } + + public function testInjectHeadersWithDarkOverrideForcesRootWithoutMedia(): void { + $themes = $this->buildThemes(); + $this->themesService->method('getThemes')->willReturn($themes); + $this->config->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn(''); + $this->themesService->method('getRequestThemeOverride')->willReturn('dark'); + + $this->service->injectHeaders(); + $collected = $this->collectHeaders(); + + $forcedDark = array_filter( + $collected['links'], + fn ($l) => $l['themeId'] === 'dark' && $l['plain'] && ($l['media'] === null || $l['media'] === ''), + ); + $this->assertCount(1, $forcedDark, 'Dark theme must be forced on :root without a media query'); + + $this->assertCount(1, $collected['metas']); + $this->assertSame('dark', $collected['metas'][0]['content']); + } + + public function testInjectHeadersDoesNotApplyOverrideWhenThemeEnforced(): void { + $themes = $this->buildThemes(); + $this->themesService->method('getThemes')->willReturn($themes); + $this->config->method('getSystemValueString') + ->with('enforce_theme', '') + ->willReturn('light'); + // Override must not even be queried when a theme is enforced + $this->themesService->expects($this->never()) + ->method('getRequestThemeOverride'); + + $this->service->injectHeaders(); + $collected = $this->collectHeaders(); + + // Media-query stylesheets remain (regular injection path) + $mediaLinks = array_filter($collected['links'], fn ($l) => $l['media'] === '(prefers-color-scheme: dark)' && $l['plain']); + $this->assertCount(1, $mediaLinks); + } +}