Missing url prefix on language neutral content

+++ modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationUrl.php
@@ -130,7 +130,7 @@
-    if (!isset($options['language'])) {
+    if (!isset($options['language']) || $options['language']->getID() === 'und') {

Any reason to change url language to url one if it was defined already?

Below are the following steps:

1. Install Content Translation module.
2. Add the Language in Configuration -> Regional and language -> Languages.
3. Click on the Detection and Selection tab next to the List tab on the languages page
4. Enable the Browser detection method and save the settings.
5. Select Enable Translation and Show language selector on create and edit pages under Language settings on edit page of content type.
6. Add translated content for the node on click the Translate option under operations for the node.
7. After add the content, on change the site in another language when click on node title it goes to node detail page with prefix url of that language.

The patch in #11 is working perfect. If we choose "-not-specified" option in Language field while creating node. On change the language of the site. The node opens with that language prefix.

It seems when we select (Language = not-specified), it will take the default path prefix code which is English in your case.
I don't think we really require a patch for (Language = not-specified). If suppose there are more than 2 languages then what should be the path prefix ?

@Patrick Bauer Good catch

I can confirm that the patch in #11 fixes the problem with language neutral content being linked to without path prefix.

The patch doesn't do anything when the language is set to not applicable, so the same buggy behaviour occurs.

This issue causes a lot of other problems with links to language neutral entities, such as media items etc:
https://www.drupal.org/node/2670144
https://www.drupal.org/project/drupal/issues/2798977
https://www.drupal.org/project/drupal/issues/2645922
https://www.drupal.org/project/drupal/issues/2889892
https://www.drupal.org/project/drupal/issues/2798977

I think this is a way better solution then adding the language inside each formatter.
I'll point the other tickets to this one, so it can be tested further.

We'll probably need to add tests for this.

I tried writing a test to fix this, but I can't find a good way to do this. I'm not sure what test-class or module this should go in. Should this be in a new test?

#11 fixes the problem for me

Fix in #11 works here too.
Thanks Patrick.

So what do we do about "not applicable"?

Wouldb be something like

 ... || $options['language']->getID() === LanguageInterface::LANGCODE_NOT_APPLICABLE

but I am not sure if we need it.

The attached patch adds support for content with language set to not applicable.

Fix provided in #11 works for me on Drupal 8.8.1

Fix in #31 worked for me (however, probably not linked to this patch, but I realized that the url was not taking the alias defined by pathauto, so I had to resort to hook_preprocess_node to rewrite the URL).

Thank you @patrick-bauer and @corneboele.
Patch #31 is working great for me.

Verified and tested the Merge request !
https://www.drupal.org/files/issues/2019-12-09/drupal-language-prefix-ne...

Testing Steps:
* Install a new Drupal instance
* Add an additional language (e.g. italian)
* Use URL negotiation and configure path prefixes (e.g. /en for english and /it for italian)
* Enable the language selector for content type
* Create new node using "not specified" as language
* Go to /it/admin/content
* The main link to the entity inside the content table should link to /it/node/1 (the current user interface language based on the url /it/admin/content).

Test Results:
Verified that after applying patch language neutral content is taking appropriate URL aliases on switching languages

Refer SS
https://www.drupal.org/files/issues/2021-09-28/Screenshot%202021-09-28%2...
https://www.drupal.org/files/issues/2021-09-28/Screenshot%202021-09-28%2...

Can be moved to RTBC
Test Status: PASS

Confirming #31 works fine and fixes the issue on Drupal 9.3.x! So RTBC+1 but still needs tests.

Setting the priority to major as this is broken "basic" functionality. The sibling issue, which I first thought was the cause: https://www.drupal.org/project/drupal/issues/2862511 is also set to major.

Where to put the tests? And which tests are expected here?

I tested #31 with current 9.3.9 - and it works as expected!!

I have read the IS and skimmed the issue. I tried to test on Drupal 10, standard install, and found that the steps to reproduce was missing an item to enable translation on content. There are also points in #16 that should be in the Issue Summary. Adding tag for an Issue Summary update. See Write an issue summary for an existing issue for guidance.

Updating the Issue Summary on this issue is a suitable first time issue, adding novice tag.

Rerolled the patch #31 for 10.0.x-dev.

Verified and tested the patch provided by #44 for 10.x dev version.
https://www.drupal.org/files/issues/2022-06-10/2883450-44.patch

Tested both the cases "Not Specified" and "Not Applicable"

Testing Steps For Not specified:

1. Install a new Drupal instance
2. Add an additional language (e.g. italian)
3. Use URL negotiation and configure path prefixes (e.g. /en for english and /it for italian)
4. Enable the language selector for the content type using this path : /admin/config/regional/content-language
5. Create new node using "not specified" as language
6. Go to /it/admin/content
7. The main link to the entity inside the content table should link to /it/node/1 (the current user interface language based on the url /it/admin/content).

Testing Steps For Not Applicable:

1. Install a new Drupal instance
2. Add an additional language (e.g. italian)
3. Use URL negotiation and configure path prefixes (e.g. /en for english and /it for italian)
4. Enable the language selector for the content type using this path : /admin/config/regional/content-language
5. Create new node using "not Applicable" as language
6. Go to /it/admin/content
7. The main link to the entity inside the content table should link to /it/node/1 (the current user interface language based on the url /it/admin/content).

Screen Shorts:

Before

After
After Applying #44 patch for not specified content

After Applying #44 patch for not applicable content

After Applying #44 patch not selecting any language in URL prefix (shows default language)

Test Results:
Verified that after applying patch language neutral content is taking appropriate URL aliases on switching languages

FYI #31 and #44 both the patches are same.

Can be moved to RTBC
Test Status: PASS

This could still use some automated test coverage, moving to needs work for that.

#2670144: Node title links to default language when language not specified was a bugsmash triage target today. I have closed it as a duplicate of this one. I have updated the Issue Summary on the suggestion made by mstrelan in Slack.

Adding test cases and test-only patches.

Oops, fixing phpcs issues and queuing again for bot.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Confirmed the issue in D10
Standard install
Enabled a 2nd language (german in my case)
Allow for content translation of basic page content type
Create a node selecting "not specified"
URL is node/1
Go to de/admin/content
URL of NodeA is /node/1
Applied patch
cleared cache
URL of NoadeA is de/node/1

Moving to NW for the issue summary update that was requested in #43

Thanks for the review @smustgrave
I've tried to attempt issue summary update. Can you please have a look at it once.

Keeping the issue tag for now, feel free to remove it after verification.

Thanks! @mohit_aghera

Looks like random failures related to CKEditor5.
Triggering for re-test and moving it to RTBC again.

Made some minor changes to the test case.

Committed 6f24730 and pushed to 10.1.x. Thanks!

Thank you.
Patch #60 seems to work in Drupal 9.

I confirm #60 apply for D9.5

That issue was breaks links that must be always neutral, like links inside sitemap.xml (if there are a lot of links the additional sitemaps are generated) and most probalby some more such cases.
Because the code was relied on "not applicable" language, but now the language applied anyway...

We're running into an issue similar to the one kala4ek described.

The OpenID Connect module generated its redirect URIs like https://default/openid-connect/generic, but now includes the language, e.g. https://default/en/openid-connect/generic

This is a huge problem for us, as it breaks the SSO login for our users when upgrading to D10.1
An issue for this hals already been created: https://www.drupal.org/project/openid_connect/issues/3383036

The OpenID Connect module generates its links as follows:

return Url::fromRoute('openid_connect.redirect_controller_redirect', $route_parameters, [
  'absolute' => TRUE,
  'language' => $this->languageManager->getLanguage(LanguageInterface::LANGCODE_NOT_APPLICABLE),
])->toString();

I'm not sure that LANGCODE_NOT_APPLICABLE should get a language as the definition of LANGCODE_NOT_APPLICABLE reads as follows:

/**
 * The language code used when the marked object has no linguistic content.
 *
 * Should be used when we explicitly know that the data referred has no
 * linguistic content.
 *
 * See http://www.w3.org/International/questions/qa-no-language#nonlinguistic.
*/
const LANGCODE_NOT_APPLICABLE = 'zxx';

I'd like to help resolve this, so if anyone knows what the correct way to generate a url without a langcode should be from now on, please let me know, so I can generate a patch for the OpenID Connect module.

Thanks for the reports. Still I think it was correct to *fix* this and I think this is a new side-effect and issue we have to discuss. Perhaps in a separate issue linking this one to understand the history?

Just an idea: Perhaps the right way would be to also provide a language neutral URL for such contents?
And eventually need a flag to determine which kind of link we need?

I think we can clearly see we can have both cases... this is a really complicated and very general task. Needs heavy discussion, I think.

In my mind it's clear that the current fix for LANG_CODE_NOT_APPLICABLE is wrong.

In the case of LANG_CODE_NOT_APPLICABLE no language prefix should be generated. No language prefix was generated before, it should not after since it will break a lot of stuff around the world. If you use LANG_CODE_NOT_APPLICABLE, you specifically don't want langcode, why would langcode be added?

See also https://www.drupal.org/project/drupal/issues/3385550, I spent an hour trying to figure out why openid_connect breaks. I will include a patch in the latter issue.

@sboden not really sure it's a safe thing, but if it is, we need a follow up which doesn't only implement that but also adds proper documentation to inform users about the important differences (propably below the selection), I think that's the most important difference then.

But let's wait for core maintainers feedback here, I think?

So what I see as a "problem" is for following code (it's a small piece from an openid_connect library):

    $language_none = \Drupal::languageManager()
      ->getLanguage(LanguageInterface::LANGCODE_NOT_APPLICABLE);

    $redirect_uri = Url::fromRoute(
      'openid_connect.redirect_controller_redirect',
      [
        'openid_connect_client' => $this->getPluginId(),
      ],
      [
        'absolute' => TRUE,
        'language' => $language_none,
      ]
    )->toString(TRUE);

Assume you don't know anything about the APIs being called and you just see this piece of code.

Do you expect "/nl" (in my case) to be added to the URL or not? That's the question and the problem I'm having. In the code above I set language explicitly to "not applicable" so I expect the "/nl" language prefix will not be added.

It did not add a language prefix before the fix, but it does now.

I also have no other easy way to get rid of the language prefix (unless we make a LANGCODE_ABSOLUTELY_NOT_APPLICABLE case, just kidding... don't do it). In my case above the generated URL is used for openid_connect and the "other side" just can't handle a language prefix.