Follow-up from #1536844-14: Port language/path handling to the kernel model. I don't know yet if this fixes test failures, but it does fix the bug reported there.

CommentFileSizeAuthor
kernel-admin-theme.patch1.25 KBeffulgentsia
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

It fixes all the problems I noticed.

- Shortcuts are back
- frontpage works
- admin theme is correct

effulgentsia’s picture

Title: system_custom_theme() incorrectly checks request path instead of system path » Fix LocalePathFunctionalTest and system_custom_theme()
Category: bug » task
Status: Reviewed & tested by the community » Needs review
+++ b/core/lib/Drupal/Core/EventSubscriber/PathListenerBase.php
@@ -16,7 +16,8 @@ use Symfony\Component\HttpFoundation\Request;
-    return $request->attributes->get('system_path') ?: trim($request->getPathInfo(), '/');
+    $path = $request->attributes->get('system_path');
+    return isset($path) ? $path : trim($request->getPathInfo(), '/');

This part fixes LocalePathFunctionalTest.

+++ b/core/modules/system/system.module
@@ -2021,7 +2021,7 @@ function system_add_module_assets() {
-    $path = ltrim($request->getPathInfo(), '/');
+    $path = $request->attributes->get('system_path');

I don't know if this fixes a test, or if we're missing test coverage for 'xx/admin' returning the admin theme if xx is a language prefix. Similarly for aliases (e.g., if 'foo' is an alias for 'admin', then 'foo' should return the admin theme).

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community
aspilicious’s picture

Title: Fix LocalePathFunctionalTest and system_custom_theme() » system_custom_theme() incorrectly checks request path instead of system path
Category: task » bug

AND it fixed the the last language fail

effulgentsia’s picture

Title: system_custom_theme() incorrectly checks request path instead of system path » Fix LocalePathFunctionalTest and system_custom_theme()

Sorry. Just noticed by looking at other issues in this project, that "bug report" is the correct category.

Lol. And sorry for the xposts :)

aspilicious’s picture

But yeah we are missing test coverage for the admin theme

Crell’s picture

Status: Reviewed & tested by the community » Fixed

I am confused why the shortened ternary did not work here, but if this fixes it, fine. We can figure that out later. :-) Committed.

effulgentsia’s picture

why the shortened ternary did not work here

Cause if you have a path like 'xx' representing the home page in language xx, then after langcode stripping, system_path is empty string, which evaluates as FALSE. In this case, we want the next listener (home page resolver) getting back the empty string, not falling back to the request path.

Crell’s picture

Ah! OK, that makes sense. So I wasn't misunderstanding the ternary, just the edge case. :-) I wonder if we should document that then so that no one tries to tidy that up later.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.