Only allowing to have empty language path prefix for the default language is inappropriate [#2411343]

Problem/Motivation

When configuring language path prefixes, there may be only one empty path prefix, that of the site default language. The intention of this is to protect users from setting an empty prefix for something that would not end up being selected as a fallback in the negotiation, basically making that language inaccessible by path. However, we made the fallback language configurable, so that this would be the site default language is not necessarily true. So you may now end up with a configuration where you cannot change the fallback language and remove the prefix before going live unless you also modify the site default language, which is silly. That was the reason we separated the default and the fallback languages to begin with.

Proposed resolution

Either only validate that at most one path prefix may be empty or validate that that is for the fallback language configured in the negotiation (not the site default).

Remaining tasks

Agree on approach. Implement.

User interface changes

Limitation on which language may get an empty prefix may change depending on configuration.

API changes

Likely none.

Comment	File	Size	Author
#15	2411343-empty_language_prefix-15.patch	4.99 KB	pcambra
#15
#15	interdiff-2411343-7-15.txt	3.54 KB	pcambra
#7	2411343-empty_language_prefix-7.patch	3.91 KB	pcambra
#7

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

andypost

he/him

Russian

CreditAttribution: andypost commented 21 January 2015 at 19:22

Language as entity could have prefix constraint

Comment #2

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 22 January 2015 at 07:48

@andypost: can you elaborate on that? I don't understand.

Comment #3

andypost

he/him

Russian

CreditAttribution: andypost commented 22 January 2015 at 08:20

I mean language is entity with prefix field that can have constraints applied
So when language entity is validated a (LanguageDefaultConstraint) constrain may check that default language is changed so prefix should be not empty or any other logic.

Comment #4

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 22 January 2015 at 10:59

Yeah IF the fallback language is still tied to the default language.

Comment #5

balagan CreditAttribution: balagan commented 6 February 2015 at 06:29

Is it a duplicate of this? https://www.drupal.org/node/338055

Comment #6

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 6 February 2015 at 07:33

No, its not.

Comment #7

pcambra

he/him

Spanish

Asturies

CreditAttribution: pcambra commented 6 February 2015 at 12:33

Status:

Active

» Needs review

File	Size
2411343-empty_language_prefix-7.patch	3.91 KB

Let's see if I'm in the right track

Comment #8

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 6 February 2015 at 13:12

Status:	Needs review	» Needs work
Issue tags:		+sprint

Looks good. Some remarks.

The UI also has a hint for the default language in the setting label. That is not yet modified here. That needs to change to "Selected fallback language" or something along those lines. I think we have a problem of using the right terminology because the selected language may be forced and not necessarily a fallback, but for the intention of the validation of this form I think that is correct to assume it would be the fallback.
The form has this help text which needs to be adjusted: For the default language, this value may be left blank

. I think "Selected fallback language" is good wording here too.

+++ b/core/modules/language/src/Tests/LanguageConfigurationTest.php
@@ -83,13 +83,16 @@ function testLanguageConfiguration() {
+    $this->assertText(t('The prefix may only be left blank for the negotiation selected language.'), 'Only negotiation language prefix can be changed to empty string.');

The feedback message is not very clear. I think we can leave that, because the text looked for is descriptive enough.

+++ b/core/modules/language/src/Tests/LanguageSwitchingTest.php
@@ -161,7 +161,7 @@ protected function doTestLanguageBlockAnonymous($block_label) {
-   * Test languge switcher links for domain based negotiation
+   * Test language switcher links for domain based negotiation

If you are fixing the grammar here, then we need a dot at the end too :)

Comment #9

balagan CreditAttribution: balagan commented 6 February 2015 at 13:20

+++ b/core/modules/language/src/Form/NegotiationUrlForm.php
@@ -142,14 +143,17 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
+          $form_state->setErrorByName("prefix][$langcode", $this->t('The prefix may only be left blank for the negotiation selected language.'));

The wording is the same here that I mentioned to Gábor on IRC, which does not make sense for me (negotiation selected language). I know that now it maches the page title, but I really don't like it.

Comment #10

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 6 February 2015 at 13:29

Yeah for this screen we may use "selected fallback language" or "selected negotiation fallback language" but TECHNICALLY it may be a negotiation provider invoked before the URL negotiation, in which case, its not the fallback. We assume its the fallback for the design of how we expect the URL negotiation to be used, but it may not be 100% true. That makes it hard to label it @balagan.

Comment #11

balagan CreditAttribution: balagan commented 6 February 2015 at 17:00

I could do with "language selected by negotiation", or "negotiated language", my problem is the word order.

Comment #12

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 9 February 2015 at 10:09

@balagan: no, those are not correct. The text refers to the language the site admin selected in the negotiation method named "Selected language". If you go to your detection and selection methods page, that has the "Selected language" option as the last one. This help text refers to the setting that the admin set there. If we want to be super specific then "The language configured for the "Selected language" negotiation method" is what this is about.

Comment #13

balagan CreditAttribution: balagan commented 9 February 2015 at 10:24

I still feel using "negotiation selected language" is wrong. Can we use "'selected language' negotiation", and on the admin/config/regional/language/detection/selected page I would change the title from "Configure selection language negotiation" to "Configure selected language negotiation". I guess I will have to open another issue for that.

Comment #14

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 9 February 2015 at 10:29

@balagan: your proposed names may or may not be good depending on the sentence you put them. Please do open an issue for the page title on that method, that needs fixing definitely.

Comment #15

pcambra

he/him

Spanish

Asturies

CreditAttribution: pcambra commented 16 February 2015 at 09:50

Status:

Needs work

» Needs review

File	Size
interdiff-2411343-7-15.txt	3.54 KB
2411343-empty_language_prefix-15.patch	4.99 KB

Let's see if this wording works better.

Comment #16

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 16 February 2015 at 10:06

Status:

Needs review

» Reviewed & tested by the community

Looks good to me, thanks!

Comment #17

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott commented 17 February 2015 at 14:26

Status:

Reviewed & tested by the community

» Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed a0add30 and pushed to 8.0.x. Thanks!

Comment #18

17 February 2015 at 14:27

alexpott committed a0add30 on 8.0.x

Issue #2411343 by pcambra: Only allowing to have empty language path...

Comment #19

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 17 February 2015 at 16:06

Issue tags:

-sprint

Yay, this is a great improvement! Thanks @pcambra!

Comment #20

3 March 2015 at 16:14

Status:

Fixed

» Closed (fixed)

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

Comment #21

mpp CreditAttribution: mpp at AmeXio for District09 commented 16 March 2018 at 15:22

This issue allows to configure an empty path prefix but it's not working on Drupal 8.5.
See https://www.drupal.org/project/drupal/issues/2953640

The test seems to cover the validation part but not the actual rendering when the prefix is empty?

Comment #22

Anybody

German

Porta Westfalica

CreditAttribution: Anybody commented 21 March 2018 at 14:37

I can confirm this problem like described in #21. We've been running into it with 8.5 now also. In our example the German language prefix is empts (/) while all other languages have a language prefix (EN: /en). If the browser language is english also the German sites with no language index return the English tests.
Interestingly the front page is a different case, it works correct.