Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Path prefix (eg. /en) is removed from incoming urls even when Part of the URL that determines language is set to Domain. To reproduce:
- Add a language (eg. German) so there are at least two.
- Visit /admin/config/regional/language/detection/url and set path prefixes. Then change Part of the URL that determines language to Domain and set the domains (eg. default (current) domain for English and some other domain for German).
- Add a node with alias looking like it's prefixed with langcode (eg. /en/test).
- Node page returns 404 because /en is removed from path by
LanguageNegotiationUrl::processInbound
.
Proposed resolution
Remove path prefix from the url only when Part of the URL that determines language is set to Path prefix.
Remaining tasks
None
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#14 | interdiff.txt | 1.05 KB | blazey |
#14 | 2855977-14.patch | 4.38 KB | blazey |
#2 | 2855977-2-test-only.patch | 2.28 KB | blazey |
Comments
Comment #2
blazey CreditAttribution: blazey at Amazee Labs commentedAttaching browser test and the fix.
Comment #5
blazey CreditAttribution: blazey at Amazee Labs commentedPathProcessorTest was not setting all required config items.
Comment #6
blazey CreditAttribution: blazey at Amazee Labs commentedComment #7
Leksat CreditAttribution: Leksat at Amazee Labs commented@blazey,
What this change does? Does it make PathProcessorTest fail? Should it go to the test-only patch then?
Comment #8
blazey CreditAttribution: blazey at Amazee Labs commented@leksat, this line actually makes PathProcessorTest pass. It didn't set all the required config items.
Comment #9
Leksat CreditAttribution: Leksat at Amazee Labs commentedAh, right. Thanks!
The code looks good and there is a test. RTBC!
Comment #11
blazey CreditAttribution: blazey at Amazee Labs commentedRerolling.
Comment #12
alexpottAs far as I can see from D7's locale_language_from_url() this patch is correct. In Drupal 7 the path was only stripped if LOCALE_LANGUAGE_NEGOTIATION_URL_PREFIX and not LOCALE_LANGUAGE_NEGOTIATION_URL_DOMAIN. So we must have broken this when we converted language negotiation to the new routing system. Going to ask a language subsystem maintainer for a +1.
Comment #13
alexpottI asked @GaborHojtsy in IRC and he agreed that this is a bug and that the fix looks correct. Nice find.
There's no need to do the explode or array_shift if the source is not LanguageNegotiationUrl::CONFIG_PATH_PREFIX. A small optimisation for sure but as we're adding the if here this seems in scope.
Comment #14
blazey CreditAttribution: blazey at Amazee Labs commentedThanks for review. Attaching updated patch.
Comment #15
blazey CreditAttribution: blazey at Amazee Labs commentedComment #16
Leksat CreditAttribution: Leksat at Amazee Labs commentedThe last change is tiny. Back to RTBC.
Comment #17
alexpottCommitted and pushed b26016d to 8.4.x and 4bd7b24 to 8.3.x. Thanks!