Problem/Motivation

In "URL language detection configuration" at admin/config/regional/language/detection/url, if you modify a path prefix for URL language detection under "PATH PREFIX CONFIGURATION" (while leaving the default language path prefix blank) an error message is generated:
The prefix may only be left blank for the selected detection fallback language.
This occurred both with English as the default installed language, and Spanish.
(As an aside, the motivation for modifying a prefix in this case was for simplified Chinese, where the default prefix zh-hans is awkward.)

Screenshot with Spanish as default installed language here.

Method to replicate:
On fresh Drupal 8 install, enable multilingual modules and add language Chinese (simplified).
Go to admin/config/regional/language/detection/url, and under "PATH PREFIX CONFIGURATION" change the path prefix for URL language detection for Chinese from zh-hans to zh, and click "Save Configuration".
Notes: Found on D8 Beta 9. This appears similar to, but different from, #2343943: Language domain may not be left blank for default language.

Proposed resolution

The error is caused by the default language code not being set correctly. Propose modifying 1 line of code with test for empty value of default language code.

Remaining tasks

None

User interface changes

None

API changes

None

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because code didn't allow admin to configure language path prefixes, and returned a wrong error message.
Issue priority Major because disallows basic language system configuration.
Unfrozen changes Not on list of unfrozen changes.
Prioritized changes A prioritized change, as it is a bugfix.
Disruption Likely no disruption, as it modifies 1 line of code logic.
CommentFileSizeAuthor
#10 modifying_path_prefix-2470307-10.patch2.52 KBeiriksm
#10 modifying_path_prefix-2470307-10-test-only.patch1008 byteseiriksm
#9 2470307-default.patch395 bytesGábor Hojtsy
#3 modifying_path_prefix-2470307-3.patch899 bytesMark_L6n
#2 modifying_path_prefix-2470307-2.patch899 bytesMark_L6n
Screen Shot 2015-04-13 at 3.17.07 PM.png184.95 KBMark_L6n
Screen Shot 2015-04-13 at 3.23.24 PM.png201.68 KBMark_L6n
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mark_L6n’s picture

Mark_L6n CreditAttribution: Mark_L6n commented
Issue summary: View changes

Found a solution, and will work on submitting a patch.
In file core/modules/language/src/Form/NegotiationUrlForm.php, variable $default_langcode was not getting set.

Mark_L6n’s picture

Mark_L6n CreditAttribution: Mark_L6n commented
Issue summary: View changes
FileSize
modifying_path_prefix-2470307-2.patch899 bytes

Uploaded patch modifying_path_prefix-2470307-2.patch.
The following line was not returning a value to $default_langcode:
$default_langcode = $this->config('language.negotiation')->get('selected_langcode');
The error was not getting caught by the subsequent test:

    if ($default_langcode == LanguageInterface::LANGCODE_SITE_DEFAULT) {
      $default_langcode = $this->languageManager->getDefaultLanguage()->getId();
    }

Testing for an empty value fixed the error:
if ($default_langcode == LanguageInterface::LANGCODE_SITE_DEFAULT || !isset($default_langcode)) {

Mark_L6n’s picture

Mark_L6n CreditAttribution: Mark_L6n commented
Issue summary: View changes
Status: Active » Needs review
FileSize
modifying_path_prefix-2470307-3.patch899 bytes

Small improvement: reversed order in || statement; probably better to have !isset($default_langcode) first.

Mark_L6n’s picture

Mark_L6n CreditAttribution: Mark_L6n commented
Issue tags: +D8MI
Mark_L6n’s picture

Mark_L6n CreditAttribution: Mark_L6n commented
Component: language system » language.module
Mark_L6n’s picture

Mark_L6n CreditAttribution: Mark_L6n commented
Issue summary: View changes
Mark_L6n’s picture

Mark_L6n CreditAttribution: Mark_L6n commented
Issue tags: +language prefix, +path prefix
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous at XIO commented
Version: 8.0.0-beta9 » 8.0.x-dev
Status: Needs review » Needs work
Issue tags: +Needs tests

I managed to reproduce this on 8.0.x, so I'm changing the version to where it will be fixed. I don't think this should be major, but since I'm not sure, I'll leave it at that.

Thank you for the patch! However, isn't this more of a work around instead of an actual fix? I wonder why the variable isn't set?

Also we will need to add some test coverage for this. So tagging accordingly.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs work » Needs review
Issue tags: +sprint
FileSize
2470307-default.patch395 bytes

We can also set the value in the default config based on the default assumption. Sounds better @pjonckiere?

Also still needs tests.

eiriksm’s picture

eiriksm
he/him
Primary language Norwegian Bokmål
Location Norway
CreditAttribution: eiriksm at Ny Media AS commented
FileSize
modifying_path_prefix-2470307-10-test-only.patch1008 bytes
modifying_path_prefix-2470307-10.patch2.52 KB

Heres a patch with a test included.

I also added the very minor patch i submitted over at #2486293: Create link to "selected detection fallback language" from error message. after discussion on IRC. This way, if people still have problems removing the path prefix, at least they get directed to the page where they can change it.

Also included a test-only patch (which should fail, I hope).

The last submitted patch, 10: modifying_path_prefix-2470307-10-test-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 10: modifying_path_prefix-2470307-10.patch, failed testing.

Status: Needs work » Needs review

eiriksm queued 10: modifying_path_prefix-2470307-10.patch for re-testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

I wanted to ask for a positive test of the message, but that seems to be already present in the existing code:

    // Change default negotiation language.
    $this->config('language.negotiation')->set('selected_langcode', 'fr')->save();
    // Check that the prefix of a language that is not the negotiation one
    // cannot be changed to empty string.
    $edit = array(
      'prefix[en]' => '',
    );
    $this->drupalPostForm(NULL, $edit, t('Save configuration'));
    $this->assertText(t('The prefix may only be left blank for the selected detection fallback language.'));

So looks great to me :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 9e69588 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation.

  • alexpott committed 9e69588 on 8.0.x 
    Issue #2470307 by Mark_L6n, eiriksm, Gábor Hojtsy: Modifying path prefix...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue tags: -sprint +language-base

Yay, thanks all!

Status: Fixed » Closed (fixed)

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

Metal3d’s picture

Metal3d CreditAttribution: Metal3d as a volunteer commented
Version: 8.0.x-dev » 8.1.3

I'm sorry but I still have this issue... Drupal core 8.1.8
Each quickedit urls still have language prefix and won't work...

Metal3d’s picture

Metal3d CreditAttribution: Metal3d as a volunteer commented
Version: 8.1.3 » 8.1.8

Changed version, sorry

tamnv’s picture

tamnv CreditAttribution: tamnv commented

Hi all,

We still have this problem with Drupal 8.2.3.