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

Comments

aspilicious’s picture

aspilicious commented
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

effulgentsia commented
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

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

aspilicious commented
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

effulgentsia commented
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

aspilicious commented

But yeah we are missing test coverage for the admin theme

Crell’s picture

Crell commented
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

effulgentsia commented

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

Crell commented

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.