betterlt can affect admin theme

Created on 3 July 2023, 11 months ago
Updated 29 April 2024, about 1 month ago

Problem/Motivation

In Drupal 8+, "admin routes" use the admin theme by default, while non-admin routes use the site's default theme by default.

But these are only defaults. Modules such as Gin Login β†’ can implement theme negotiators that override Drupal's default behavior.

That means that when better_local_tasks_theme_registry_alter() checks the admin route context, it is not actually checking whether or not the admin theme is being used on the current page. On a site with theme negotiators, this can result in betterlt's theme registry altering condition being met for the admin theme.

Steps to reproduce

  1. Install Better Local Tasks, Gin β†’ , and Gin Login β†’ .
  2. Set Gin as the administrative theme.
  3. Clear Drupal's cache. As your first request to the site after the cache clear, hit /user/login as an unauthenticated user. (Use an incognite window, curl, etc.)
  4. Log back in to Drupal and browse the Gin admin theme. You'll see betterlt markup appearing in the admin theme whenever local tasks are rendered.

Proposed resolution

In the theme registry alter hook, check Drupal's admin theme setting in system.theme. Alter the theme registry unless the current theme is the admin theme.

It should be noted this will prevent betterlt from working at all on a site using the same default theme for frontend & backend. But I think that's actually slightly better than the current behavior. Right now on such sites, betterlt would be non-deterministically either always on or always off for the duration of the theme registry being cached, depending on what route happens to trigger the theme registry being built. This would at least change the behavior to deterministically never using the betterlt template.

The module's current method of attaching its asset library suffers from the same problem. The MR in #3371119 πŸ› betterlt has undeclared dependency on "contextual" module and specific user configuration Needs review fixes this issue as well as the issue noted there.

Remaining tasks

  1. Fix #3371119 πŸ› betterlt has undeclared dependency on "contextual" module and specific user configuration Needs review
  2. Fix the theme registry hook.
πŸ› Bug report
Status

Needs review

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States bvoynick

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @bvoynick
  • @bvoynick opened merge request.
  • Status changed to Needs review 11 months ago
  • πŸ‡ΊπŸ‡ΈUnited States bvoynick
  • πŸ‡¦πŸ‡·Argentina gerzenstl Resistencia

    I'm getting same problem on a site that only has Better Local Tasks β†’ and Gin β†’ , but I can't find a good way to reproduce it consistently.

    I do believe it is related in the way that better_local_tasks_theme_registry_alter() sometimes fails to override the template in pages that aren't admin page. Using router.admin_context service is fine, many contrib and core module use it to check if a page belongs to an admin route.

Production build 0.69.0 2024