Propagate current locale to custom 404/403/500 error pages#3726
Open
Vondry wants to merge 2 commits into
Open
Conversation
206c1ca to
1222eef
Compare
When no route matches (404/403), Symfony's LocaleListener never runs, so custom error pages and their records rendered in the default locale and ignored the locale in the URL (e.g. /nl/...). - ErrorController recovers the locale from the URL path and applies it to the request, the Twig sub-request, and the translator; the record locale is now passed explicitly to DetailController::record(). - Add redirectToDefaultLocaleOrFallback(): when there's no route to redirect to (error page / forwarded request), reset to the default locale and render instead of erroring. Guards the missing _route case that previously threw a TypeError (a 404-within-a-404). - ListingController uses the fallback so a forwarded listing renders in the default locale instead of erroring. Adds ErrorControllerTest and ListingControllerTest covering localized and default-locale rendering, translator locale recovery, the 403 path, the non-localized ContentType regression, the listing redirect, and the forwarded-listing fallback.
1222eef to
0590e45
Compare
On PHP 8.4, mb_trim()/mb_rtrim() are analysed as string|false (the mbstring polyfill's implementation has no return type, and the native 8.4 stubs are nullable-on-failure). Combined with json_encode()'s existing string|false return, this surfaced level-8 errors in files unrelated to any one feature. Add explicit (string) casts at the call sites in Canonical, the translation trait, the frontend menu builder, the content repository search, and the excerpt helper. These are no-ops on PHP 8.2/8.3 and satisfy the analyser on 8.4. Drop the now-obsolete ContentRepository mb_trim baseline entry, which the json_encode cast resolves.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Propagate the current locale to custom 404 / 403 / 500 error pages
Problem
When a request hits a custom error page configured in
config/bolt/config.yaml(
notfound,forbidden,internal_server_error,maintenance), the page wasalways rendered in the default locale, even when the URL clearly carried a
locale (e.g.
/nl/this-page-does-not-exist).Root cause: on a 404/403, no route matches, so Symfony's
LocaleListenerneverruns and the request locale is never set from the URL. As a result:
heading) rendered in the default language.<html lang="…">did not reflect the requested locale.{% trans %}strings in the error template rendered in the default language(the translator's locale was never set either).
There was also a latent bug: forcing the "wrong locale" path through
redirectToDefaultLocale()on a request without a matched route calledredirectToRoute(null, …), throwing aTypeError— a 404-within-a-404.Solution
Recover the locale from the URL path on error pages.
ErrorControllernow receives the configured locales (%app_locales%) and,in
showAction(), recovers the locale from the first path segment(
/de/…⇒de) before rendering. It's applied to:app.requestresolves to, and{% trans %}strings localize too).The translator call is guarded by
instanceof LocaleAwareInterface: thecontracts
TranslatorInterfacewe depend on exposes onlytrans()/getLocale(), whilesetLocale()lives onLocaleAwareInterface(theconcrete translator implements both). This keeps the declared dependency
narrow.
Pass the locale explicitly into record rendering.
attemptToRender()now forwards$request->getLocale()toDetailController::record(), so it keeps the recovered locale instead offalling back to the default.
Make the "wrong locale" handling degrade gracefully.
Introduced
redirectToDefaultLocaleOrFallback(): when there is a matchedroute it still redirects to the default-locale URL as before; when there
isn't one (an error page, or a forwarded request) it resets the request to
the default locale and returns
null, so the caller renders in the defaultlocale instead of erroring.
redirectToDefaultLocale()now guards the missing_routecase (fixing theTypeError) and reads_routefrom requestattributes rather than
$request->get().Changes
src/Controller/ErrorController.php%app_locales%and the translator; recover the locale from the path (setLocaleFromPath()) onto the request, sub-request andDetailController::record().src/Controller/TwigAwareController.phpredirectToDefaultLocaleOrFallback(); guard the no-_routecase inredirectToDefaultLocale()and read_routefromsrc/Controller/Frontend/ListingController.phptests/php/Controller/Frontend/ErrorControllerTest.phptests/php/Controller/Frontend/ListingControllerTest.phpTests
ErrorControllerTest:testNotFoundRecordRendersLocalizedFieldInRequestedLocale—/nl/…404 renders<html lang="nl">and the Dutchheading.testNotFoundRecordRendersLocalizedFieldInDefaultLocale— no-locale 404 rendersin
en.testErrorPageRecoversTranslatorLocaleFromPath— the shared translator's localeis set to
nlfor a/nl/…error (default isen).testForbiddenRecordRendersLocalizedFieldInRequestedLocale— the 403 branch(
showForbidden) localizes the configuredgeneral/forbiddenrecord.testNotFoundPageWithNonLocalizedContentTypeDoesNotError— regression for theTypeErrorwhen a non-localized ContentType is reached via a localized URL.ListingControllerTest:testListingRedirectsToDefaultLocaleWhenLocaleNotSupported— matched route withan unsupported locale ⇒ 302 to the default locale.
testListingFallsBackToDefaultLocaleWhenForwardedWithoutRoute— forwardedrequest (no
_route) ⇒ renders in the default locale instead of erroring.Results: new tests green (
ErrorControllerTest5/5,ListingControllerTest2/2). Full suite 195/196 — the single failure (
ConfigTest::testCanParse, aConsole\Application::setDispatcher(null)TypeError) is a pre-existingtest-ordering issue unrelated to this change: it touches none of these files and
passes in isolation. PHPStan and ECS are clean on all changed files.
Notes / limitations
in the test app (all
access_controlrules are backend, and the backend 403path redirects to the dashboard rather than rendering the custom page), so that
test drives the configured
error_controllerdirectly — the same entry pointSymfony's error handling uses.
showAction()returns the rendered page as aplain 200; promoting the status to the exception's code (403/404/…) is the
kernel's job (
HttpKernel::handleThrowable()), which the direct call bypasses —so that test asserts the locale handling that is this controller's
responsibility, not the status code.
<html lang>may showthe URL locale while the record content uses the default locale. This is
harmless (such content is identical across locales) and is documented inline.
ErrorController::__constructgains a requiredstring $localesargument and aTranslatorInterface; both are autowired/boundvia DI (
config/services.yaml). Relevant only if the controller is extended orinstantiated manually.