Modern-stack controllers bypass the theme cascade (hardcoded /default/, CssHookProvider has no implementation)

[pierrefardel]

Themed CSS is bypassed on the new controller-based pages

Branch FRAMEWORK_6_0, observed on a live install (Horde 6, PHP 8.4).

Four pages ignore the user's theme and render with default:

• /horde/settings/oauth/
• /horde/admin/authentication/provider/
• /horde/admin/authentication/status/
• /horde/admin/apis/

There are two separate causes. I have a patch for the first (inlined at the
end); the second is an architectural question I would rather ask than answer.

Side context: back in August, on Core#222, you mentioned the modern stack
should be exercised by whups, wicked and the modernized smartmobile views —
none of which we have deployed, which is why I could not test that PR at
runtime. These four pages turn out to be the first modern-stack pages I can
actually reach on our install: they use Horde\Core\PageOutput\PageComposer
and AssetCollector rather than Horde_PageOutput. So this report doubles as
the runtime feedback I owed you — the stack itself renders fine, it is the CSS
side that has the gaps below.

1. The theme path is hardcoded

Five controllers build the stylesheet URI by hand:

$themesUri = $this->registry->get('themesuri', 'horde');
$this->assetCollector->addStylesheet($themesUri . '/default/screen.css');
$this->assetCollector->addStylesheet($themesUri . '/default/settings.css');

• base/src/Settings/OAuthAccountController.php:466
• base/src/Admin/ApiRegistryController.php:142
• base/src/Admin/OAuthSystemStatusController.php:123
• base/src/Admin/OAuthProviderController.php:395
• base/src/Admin/AdminDashboardController.php:153 (admin-dashboard.css)

/default/ is literal, so these pages stay on the default theme whatever the
user selected. AssetCollector::addStylesheet() only takes a raw URL, so the
controllers had no theme-aware call available — which is presumably why the
path was inlined.

CascadeCssDiscoverer already does exactly the right thing: it walks
horde/default → horde/<theme> → <app>/default → <app>/<theme> and
emits only files that exist, so a theme that does not ship settings.css
still inherits the default one.

The patch below adds ViewModeConfigurator::addThemeStylesheets() — the
same service that already populates AssetCollector with scripts — and has
the five controllers call it. The new constructor arguments are optional and
default to null, so the method is a no-op if the services are unavailable.

Verified on the live install: the page now requests
/themes/horde/upjv/screen.css (HTTP 200) alongside the default files.

2. CssHookProvider has no implementation

With the patch applied, the page loads the theme's screen.css but still
renders unstyled — worse than before, because it has left the complete
default theme for a theme it cannot fully resolve.

Our screen.css is an entry point that pulls in 21 files via @import, and
all of them depend on custom properties defined in tokens.css plus a CDN
stylesheet. Both are injected by the cssfiles() hook, which
Horde_Themes_Css::getStylesheets() calls (Core/lib/Horde/Themes/Css.php
l.174-186). Without those variables every rule resolves to nothing.

CascadeCssDiscoverer accepts a CssHookProvider and calls getHookFiles()
(l.44-48), and CssDiscovererFactory tries to resolve one (l.44-48) — but
CssHookProvider is an interface with no implementation anywhere in the
tree:

Core/src/Assets/CssHookProvider.php:19      interface CssHookProvider
Core/src/Assets/CascadeCssDiscoverer.php:29 private readonly ?CssHookProvider $hookProvider = null
Core/src/Factory/CssDiscovererFactory.php:45 $injector->getInstance(CssHookProvider::class)

The try/catch swallows the failure and passes null, so the modern cascade
silently never runs cssfiles().

There is a related divergence: Horde_Themes_Cache honours a theme's
$theme_covers declaration to skip the default-theme fallback per app
(Themes/Cache.php l.387-390), while CascadeCssDiscoverer has no notion of
it and always layers default underneath. A theme that declares full coverage
gets both stylesheets on these pages and only its own everywhere else.

Questions

1. Is CssHookProvider intended to get an implementation that bridges
   cssfiles(), or is dropping hook support on the new cascade deliberate?
2. Should CascadeCssDiscoverer learn about $theme_covers, or is the
   always-cascade behaviour the intended replacement?

I am happy to write either patch once the intent is settled. Applying the one
below on its own would regress any theme that relies on the hook, so it
probably should not land alone — hence an issue rather than a pull request.

Patch for cause 1

Against horde/Core and horde/base, FRAMEWORK_6_0. Not submitted as a PR
for the reason above.

Diff (7 files, +72/-22)

--- a/Core/src/PageOutput/ViewModeConfigurator.php	2026-09-23 10:42:27
+++ b/Core/src/PageOutput/ViewModeConfigurator.php	2026-09-23 11:01:32
@@ -16,7 +16,10 @@
 
 namespace Horde\Core\PageOutput;
 
+use Horde\Core\Assets\CssDiscoverer;
+use Horde\Core\Assets\CssDiscoveryRequest;
 use Horde\Core\Assets\JsDiscoverer;
+use Horde\Core\Assets\ThemeResolver;
 use Horde\Core\Service\PrefsService;
 use Horde\Core\Session\HordeSession;
 use Horde\Core\Session\SessionAccess;
@@ -45,8 +48,46 @@
         private readonly SessionAccess $session,
         private readonly JsDiscoverer $jsDiscoverer,
         private readonly Token $tokenService,
+        private readonly ?CssDiscoverer $cssDiscoverer = null,
+        private readonly ?ThemeResolver $themeResolver = null,
     ) {}
 
+    /**
+     * Adds themed stylesheets to the output.
+     *
+     * Resolves the user's theme and walks the asset cascade, so a theme that
+     * does not ship $files still gets the default theme's version, and one
+     * that does gets it layered on top.
+     *
+     * Controllers outside the Horde_PageOutput path used to hardcode
+     * '/default/' in the stylesheet URI, which pinned those pages to the
+     * default theme whatever the user had selected.
+     *
+     * @param list<string> $files Stylesheet names, e.g. ['screen.css'].
+     */
+    public function addThemeStylesheets(
+        AssetCollector $collector,
+        array $files,
+        string $app = 'horde',
+    ): void {
+        if ($this->cssDiscoverer === null || $this->themeResolver === null) {
+            return;
+        }
+
+        $uid = $this->session->getAuthId() ?? '';
+        $theme = $uid === ''
+            ? 'default'
+            : $this->themeResolver->resolve($uid, null, $app);
+
+        $result = $this->cssDiscoverer->discover(
+            new CssDiscoveryRequest(files: $files, app: $app, theme: $theme)
+        );
+
+        foreach ($result as $entry) {
+            $collector->addStylesheet($entry->uri);
+        }
+    }
+
     public function configure(AssetCollector $collector, ViewMode $mode): void
     {
         $this->addBasicScripts($collector);
--- a/Core/src/PageOutput/ViewModeConfiguratorFactory.php	2026-09-23 10:42:27
+++ b/Core/src/PageOutput/ViewModeConfiguratorFactory.php	2026-09-23 10:42:06
@@ -16,7 +16,9 @@
 
 namespace Horde\Core\PageOutput;
 
+use Horde\Core\Assets\CssDiscoverer;
 use Horde\Core\Assets\JsDiscoverer;
+use Horde\Core\Assets\ThemeResolver;
 use Horde\Core\Service\PrefsService;
 use Horde\Core\Session\SessionAccess;
 use Horde_Injector;
@@ -42,6 +44,8 @@
             $injector->getInstance(SessionAccess::class),
             $injector->getInstance(JsDiscoverer::class),
             $injector->getInstance(Token::class),
+            $injector->getInstance(CssDiscoverer::class),
+            $injector->getInstance(ThemeResolver::class),
         );
     }
 }
--- a/base/src/Settings/OAuthAccountController.php	2026-09-23 10:42:27
+++ b/base/src/Settings/OAuthAccountController.php	2026-09-23 10:42:06
@@ -463,9 +463,10 @@
 
     private function renderChrome(string $title, callable $renderBody): string
     {
-        $themesUri = $this->registry->get('themesuri', 'horde');
-        $this->assetCollector->addStylesheet($themesUri . '/default/screen.css');
-        $this->assetCollector->addStylesheet($themesUri . '/default/settings.css');
+        $this->configurator->addThemeStylesheets(
+            $this->assetCollector,
+            ['screen.css', 'settings.css'],
+        );
         $this->configurator->configure($this->assetCollector, ViewMode::BASIC);
 
         $meta = new PageMeta(title: $title);
--- a/base/src/Admin/ApiRegistryController.php	2026-09-23 10:42:27
+++ b/base/src/Admin/ApiRegistryController.php	2026-09-23 10:42:06
@@ -139,9 +139,10 @@
 
     private function renderChrome(string $title, callable $renderBody, string $currentUrl): string
     {
-        $themesUri = $this->registry->get('themesuri', 'horde');
-        $this->assetCollector->addStylesheet($themesUri . '/default/screen.css');
-        $this->assetCollector->addStylesheet($themesUri . '/default/settings.css');
+        $this->configurator->addThemeStylesheets(
+            $this->assetCollector,
+            ['screen.css', 'settings.css'],
+        );
         $this->configurator->configure($this->assetCollector, ViewMode::BASIC);
 
         $meta = new PageMeta(title: $title);
--- a/base/src/Admin/OAuthSystemStatusController.php	2026-09-23 10:42:27
+++ b/base/src/Admin/OAuthSystemStatusController.php	2026-09-23 10:42:06
@@ -120,9 +120,10 @@
 
     private function renderChrome(string $title, callable $renderBody, string $currentUrl): string
     {
-        $themesUri = $this->registry->get('themesuri', 'horde');
-        $this->assetCollector->addStylesheet($themesUri . '/default/screen.css');
-        $this->assetCollector->addStylesheet($themesUri . '/default/settings.css');
+        $this->configurator->addThemeStylesheets(
+            $this->assetCollector,
+            ['screen.css', 'settings.css'],
+        );
         $this->configurator->configure($this->assetCollector, ViewMode::BASIC);
 
         $meta = new PageMeta(title: $title);
--- a/base/src/Admin/AdminDashboardController.php	2026-09-23 10:42:27
+++ b/base/src/Admin/AdminDashboardController.php	2026-09-23 10:42:06
@@ -150,9 +150,10 @@
 
     private function renderChrome(string $title, callable $renderBody): string
     {
-        $themesUri = $this->registry->get('themesuri', 'horde');
-        $this->assetCollector->addStylesheet($themesUri . '/default/screen.css');
-        $this->assetCollector->addStylesheet($themesUri . '/default/admin-dashboard.css');
+        $this->configurator->addThemeStylesheets(
+            $this->assetCollector,
+            ['screen.css', 'admin-dashboard.css'],
+        );
         $this->configurator->configure($this->assetCollector, ViewMode::BASIC);
 
         $meta = new PageMeta(title: $title);
--- a/base/src/Admin/OAuthProviderController.php	2026-09-23 10:42:27
+++ b/base/src/Admin/OAuthProviderController.php	2026-09-23 10:42:06
@@ -392,9 +392,10 @@
 
     private function renderChrome(string $title, callable $renderBody, string $currentUrl): string
     {
-        $themesUri = $this->registry->get('themesuri', 'horde');
-        $this->assetCollector->addStylesheet($themesUri . '/default/screen.css');
-        $this->assetCollector->addStylesheet($themesUri . '/default/settings.css');
+        $this->configurator->addThemeStylesheets(
+            $this->assetCollector,
+            ['screen.css', 'settings.css'],
+        );
         $this->configurator->configure($this->assetCollector, ViewMode::BASIC);
 
         $meta = new PageMeta(title: $title);

Side note

Horde/Core/Form/daterenderer (~l.323) calls Horde::img('ratio.png') while
every other call in the same block uses the image/ prefix, so the file never
resolves and the src comes out empty. One-word fix, happy to send it
separately.

[pierrefardel]

Follow-up with a case that isolates cause 2 from cause 1.

/horde/portal/ (ResponsivePortalController) resolves the theme correctly —
no hardcoded /default/ here. ResponsiveAssets::getCssUrls() walks the proper
cascade and the browser does request /themes/horde/upjv/screen.css, plus the
files it @imports.

The page still renders unthemed.

That rules out the hardcoded path as the root cause: our screen.css arrives,
but its rules resolve to nothing because the custom properties they rely on
(tokens.css and a CDN stylesheet) are injected by the cssfiles() hook, which
no modern-stack path ever calls.

So the patch in the issue body fixes a real bug but would not make a single one
of these pages themed on its own. CssHookProvider having no implementation is
the blocker, and it affects every modern-stack page, not just the four listed.

This one matters more than the admin pages, since the class docblock says it
"replaces the traditional portal view and smartmobile view with a unified
responsive design" — it is meant to become the home page.

Unrelated question about that page

While testing I noticed ResponsivePortalController renders no topbar and no
sidebar, unlike the other modern-stack controllers which call TopbarRenderer.
The result is a standalone launcher: once you are on it, the only way out is to
click an application.

It also replaces /services/portal/, which is block-based and user-customisable
(weather, tasks, recent mail, …). Is that customisation meant to come back in the
responsive portal, or is the new page deliberately a simple launcher? Happy to
open a separate issue if you prefer.

Two smaller things in templates/portal/responsive.html.php, if useful: the
<title> and the "Your Applications" heading are hardcoded English rather than
going through gettext, and the page is served with <html lang="en"> regardless
of the user's language.