Problem/Motivation

- Path module doesn't take into consideration the language when using session for detection, when using url prefix it works.
- Translation switch links in node's view doesn't output the alias of the translated nodes, it just print a plain url (e.g. node/3 instead of node/alias)

Steps to reproduce

  • Add a language, e.g. Spanish (es), detect the language using session.
  • Add a node, translate it, and set the translated node alias to something 'foo'
  • View the original node, the href of translated node link appears without the alias (e.g. node/2).
  • Try manually to go to 'foo', doesn't work.

Proposed resolution

- Allow Path module to detect the language from the global $language variable instead of $language_url
- Edit translation_language_switch_links_alter() to make it use url() instead of forming hard coded url.

Remaining tasks

Write a test to show how it failed to request a path alias for translated node when using session to detect the language, and to show how translated links are not using aliases.

User interface changes

None.

API changes

None.

Original report by [username]

Vapes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

walkah’s picture

Just a quick note that I'm seeing this as well. It seems to be an issue with content translation other pages (views, etc) switch languages just fine.

URL detection (both path and subdomain) seem to work.

Countzero’s picture

Just filled a bug in the Entity Translation queue about this : http://drupal.org/node/1345716

Not sure if I should have filled it here instead, so cross linking.

Countzero’s picture

Version: 7.8 » 7.10

I can confirm this bug is still present in 7.10, and is not related to ET.

Steps to reproduce :

- Enable Entity and Entity Translation
- Set the detection method to Session
- Create a node and give it an alias
- Translate it and give the translation an alias

No version of the node is accessible via its alias, not even the one in the original language.

I could try to investigate this if I had a hint about the code involved, which I'm trying to find by myself at this very moment.

plach’s picture

good_man’s picture

Version: 7.10 » 7.x-dev
Assigned: Unassigned » good_man
Status: Active » Needs work

I wrote a patch that solve this, but first I want to write a test that detect this bug, otherwise the patch won't be committed :) Any one can confirm it on D8?

plach’s picture

Version: 7.x-dev » 8.x-dev

The D8 language negotiation code is almost identical to the D7 one so this bug should be present in D8 too.

good_man’s picture

Issue summary: View changes

Put the details into template.

Countzero’s picture

goodman, I'd be more than happy to give your patch a try.

Besides, I hereby withdraw from the hunt on this bug, as work seems to already be done.

good_man’s picture

Status: Needs work » Needs review
FileSize
3.63 KB

I think this is related #269877: path_set_alias() doesn't account for same alias in different languages although I didn't read it fully yet.

good_man’s picture

A patch passing the test case.

Status: Needs review » Needs work

The last submitted patch, session_detection_1294946_9.patch, failed testing.

good_man’s picture

Patch in #8 meant to be failed, but in #9 not. So it needs a new patch for the failed test in path module.

Countzero’s picture

I tried to apply the changes manually to 7.10 and it doesn't seem to work : still got the 404 on every version of the node, including original.

good_man’s picture

Issue tags: +D8MI

from path.test:

// Change user language preference.
$edit = array('language' => 'fr');
$this->drupalPost("user/{$this->web_user->uid}/edit", $edit, t('Save'));

// Check that the English alias works. In this situation French is the
// current UI and content language, while URL language is English (since we
// do not have a path prefix we fall back to the site's default language).
// We need to ensure that the user language preference is not taken into
// account while determining the path alias language, because if this
// happens we have no way to check that the path alias is valid: there is no
// path alias for French matching the english alias. So drupal_lookup_path()
// needs to use the URL language to check whether the alias is valid.
$this->drupalGet($english_alias);
$this->assertText($english_node->title, 'Alias for English translation works.');

hmm not sure why path module has to determine the language based only on $language_url. In this case (user preference) and the issue's case (session), they both write the language to $language (so does the url detection). So why not using $language instead of $language_url? In this case we need to edit this comment and path test case.

good_man’s picture

@Countzero: are the steps you did the same of this issue? take a look into url aliases (if you enable path module go to URL Aliases page) check the alias(es) you have for this node, make sure they have the right language/path, then try again with the patch.

Countzero’s picture

FileSize
19.37 KB

Yes, the languages and aliases are OK, as they were before applying the modifications.

Please see attachment to check.

plach’s picture

URL aliases are not tied to the interface language, their language is determined only by their prefix, i.e. what the URL method looks for.

If your UI language is french, you are visiting a URL like http://example.org/it/contenuto, and french is used to look up an alias you will get a 404, since no french alias exists for the path '/contenuto' .

Session and URL methods are mutually exclusive wrt URL rewriting, so using Session language for aliases only makes sense when there is no prefix or domain information, e.g.:

http://example.org/contenuto?language=it

The responsible for initializing URL language in this case is http://api.drupal.org/api/drupal/includes--locale.inc/function/locale_la....

Countzero’s picture

My bad : the aliases definitely work, but I had to add the session variable to the URL.

My apologies for the crappy testing feedback.

So, good_man's patch above works, in a way.

But I understand that as of now, there's no way to access an aliased translation with session based negociation. So there's work left on the issue, right ?

good_man’s picture

You can click either on the translated link shown at the bottom of the original node, or enable the language switcher block (from blocks page) and click on different links on it when viewing a node, so for example if you are viewing an English node, and you click French, Drupal will redirect to the French version of this node. Hope this help answers your question.

@plach: What do you think about the question I raised in #13? should we rely on $language variable everywhere? as it holds the current language in different scenarios better than $language_url.

Countzero’s picture

Well, in fact, no, it doesn't answer my question.

I don't want to spam this issue, but to make it short, my use case doesn't fit with these functionalities : I need the nodes to be accessible via their legacy URL, which lead to static files before the site was ported to Drupal. I think it's one major reason for using the session based negociation : keeping URLs as they are, and being able to access any page directly in any language.

Besides, I think it was (at least one of) the point(s) raised by the OP. Whatever the implementation is as of now, as soon as the translations are aliasable, one expects to be able to access them directly via their alias, without messing with other stuff.

But perhaps it's more of a feature request than a bug report, this I leave to your judgment.

plach’s picture

@plach: What do you think about the question I raised in #13? should we rely on $language variable everywhere? as it holds the current language in different scenarios better than $language_url.

#16 was exactly an answer to #13: we cannot use $language for the reasons explained there, what if also $language_content is configurable (i.e. it may differ from $language)? Should we use content or UI language to lookup URLs? There is no right answer, probably it depends on the use cases.

What we can do is fixing $language_url in case no prefix or domain is present: currently in this case the default language is returned, this is correct only when the URL language method is enabled (which is the check we are missing in locale_language_url_fallback), otherwise $language is a sensible fallback.

Besides, I think it was (at least one of) the point(s) raised by the OP. Whatever the implementation is as of now, as soon as the translations are aliasable, one expects to be able to access them directly via their alias, without messing with other stuff.

Yes, I was not arguing against this, see the last example in #16.

Edit: fixed typos all around.

Countzero’s picture

Yes, I know, I was answering good_man.

The function you point in #16 doesn't know about the entity concerned. It just checks the environment variables.

For the functionality we're talking about, the entity translation language trying to be accessed via the current path has to be checked in a way or another.

Here's the result of my poor tries.

I added the folowing code in locale_language_url_rewrite_session :

  $alias = drupal_get_path_alias($path);
  $info = path_load(array('alias' => $alias));
  dsm($alias . '?language=' . $info['language']); // This message outputs the correct URL, with the language variable appended, with the correct language value for the requested alias
  $options['query']['language'] = $info['language'];

Requesting the page with the alias only (in the form "www.thesite.com/index-EN.html) correctly outputs the formatted URL in the messages area, but doesn't change the actual page URL, for a reason I didn't understood yet.

Am I on the right track ?

plach’s picture

This one might work, let's ask the bot.

plach’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, session_detection-1294946-22.patch, failed testing.

Countzero’s picture

For people like me who don't really understand the insides of core path management, here is a probably very bad workaround which I'll use on a demo site :

function whatever_module_init() {
   $path = request_path();
   if (! isset($_REQUEST['language'])) {
      $alias = drupal_get_path_alias($path);
      $info = path_load(array('alias' => $alias));  
      $options = array ('query' => array('language' => $info['language']));  
      drupal_goto($path, $options);
   } 
}

It's probably a very bad thing to do, but it works so far. Mean : it retrieves the node with the alias entered in the address bar and appends the session language variable with the correct value from the real node's translation.

So, if http://thesite.com/my-alias-to-russian-node is the page requested, and given the corresponding node's translation alias is the russian version, the redirect will lead to http://thesite.com/my-alias-to-russian-node?language=ru.

Hope it can help some until this issue is solved.

timofey’s picture

Here's another very, very important related issue.

An aliased URL with a language prefix does not return content.
If you translate body & title fields of a node, using Entity Translation module, both languages will be sharing the same NID. So if I created content in 'en', and translated into 'ru', 'de', 'es'.... I will not be able to access content through ru.example.com/aliaseduri/abc, de.example.com/aliaseduri/abc, es.example.com/aliaseduri/abc... but en.example.com/aliaseduri/abc works. That is because drupal_path_initialize() grabs aliased url by uri and language.

As a temporary fix, I insert this
$path_language == 'ru' ? $path_language = 'en' : '';
into include/path.inc right under $path_language = $path_language ? $path_language : $language->language;
But it only fixes the problem if the node was originally created in 'en'...

I hope I made sense!

Gábor Hojtsy’s picture

Title: Locale's Language Detection based on Session doesn't work with URL Aliases » Language detection based on session doesn't work with URL aliases
Issue tags: +language-base

Tagging for base language system.

Gábor Hojtsy’s picture

Issue tags: +negotiation

Tagging for language negotiation too.

fenda’s picture

Any progress with this?

cweagans’s picture

Gábor Hojtsy’s picture

Issue tags: +sprint

Put on the sprint as an interesting and well located bug :)

ygerasimov’s picture

Here is rerolled patch with test only

sxnc’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, core-1294946-node-url-aliases-testonly.patch, failed testing.

Kristen Pol’s picture

+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
@@ -68,7 +68,7 @@ class TranslationTest extends WebTestBase {
-  function testContentTranslation() {
+  function t1estContentTranslation() {
     // Create Basic page in English.

I assume the "1" in "t1est" is a typo?

+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
@@ -152,7 +152,7 @@ class TranslationTest extends WebTestBase {
-  function testLanguageSwitchLinks() {
+  function t1estLanguageSwitchLinks() {
     // Create a Basic page in English and its translation in Spanish.

Same... I assume the "1" in "t1est" is a typo?

+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
@@ -190,7 +190,7 @@ class TranslationTest extends WebTestBase {
-  function testLanguageSwitcherBlockIntegration() {
+  function t1estLanguageSwitcherBlockIntegration() {
     // Add Italian to have three items in the language switcher block.

One more... I assume the "1" in "t1est" is a typo?

Nothing else is jumping out at me ;)

ygerasimov’s picture

Yes, sorry guys. Adding 1 in the name of functions was just way to run only one test case. This should be removed.

vasi1186’s picture

Component: language system » token system
Status: Needs work » Needs review
FileSize
3.57 KB

Attached a patch that just removed the 1's in the previous one.

Status: Needs review » Needs work

The last submitted patch, language_detection_1294946_37.patch, failed testing.

Kristen Pol’s picture

+++ b/core/modules/translation/lib/Drupal/translation/Tests/TranslationTest.php
@@ -249,6 +249,59 @@ class TranslationTest extends WebTestBase {
+    // Get the english node to check the translated node link.
+    $this->drupalGet('node/' . $node->nid);

I didn't notice this before, but to be nit-picky, "english" should be capitalized.

I went through all the comments and code and it seems pretty understandable although I don't know much about simpletest.

webflo’s picture

Status: Needs work » Needs review
FileSize
1.64 KB
4.43 KB

Fixed the typo from 39 and rerolled 22.

Status: Needs review » Needs work

The last submitted patch, language_detection_1294946_40.patch, failed testing.

Kristen Pol’s picture

Status: Needs work » Reviewed & tested by the community

This looks good to me. It needs more people reviewing though.

marcusx’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
4.14 KB

As far as I can tell #22 won't fix it.

Patch with test, typo from #39 fixed and a new fix.

-> triggering bot

Status: Needs review » Needs work
Issue tags: -Needs backport to D7, -D8MI, -sprint, -language-base, -negotiation
marcusx’s picture

marcusx’s picture

Started tests that were ended with "Fatal Error" locally with no problems.

  • AggregatorConfigurationTest.php
  • LoopTest.php

Trying to retest.

Status: Needs review » Needs work
lazysoundsystem’s picture

#40 was working except for the 'translated node switch' test.

Turns out this was looking for the link to 'Español', but the test doesn't install the .po file, so the link text remains 'Spanish'.

With this change tests are passing (locally, at least).

lazysoundsystem’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, language_detection_1294946_48.patch, failed testing.

lazysoundsystem’s picture

#48 fixed the Translation test, but it's still failing the Path and Locale tests. I'm checking those now.

marcusx’s picture

Component: token system » language system
FileSize
4.78 KB

So here is a new patch from todays work at the #D8MI Sprint. We couldn't finish it as people had to leave but we are on a good way.

The language tests are woking now. And we have the desired behaviour. But now two other tests are broken - we need to look into this a little more.

marcusx’s picture

Status: Needs work » Needs review

Lets check if the testbot brings the same results than we got locally.

Status: Needs review » Needs work
marcusx’s picture

damm it, on my local installation only the comment tests where broken. I don't get it. :-(

Gábor Hojtsy’s picture

Issue tags: -sprint

This is a regular bug that is not subject to new feature development as far as we found so far, so I don't think I should keep pointing people to this vs. other things that are feature freeze blockers. Therefore removing from sprint.

jair’s picture

Issue tags: +Needs reroll

Needs reroll

jair’s picture

Issue summary: View changes

es == Spanish.

generalredneck’s picture

I've started the d7 backport initiative for this problem over in #2156113: D7: Language detection based on session doesn't work with URL aliases so that there won't be version switching in this issue.

alansaviolobo’s picture

re-rolled the patch.

alansaviolobo’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 59: language_detection-1294946-59.patch, failed testing.

sdelbosc’s picture

Sorry if I introduce some confusion here but I wonder if there is any good reason to not have a simpler language_url_fallback() function.
I was thinking of something like this:

function language_url_fallback($language = NULL, $request = NULL, $language_type = Language::TYPE_INTERFACE) {
  return language($language_type)->id;
}

Any thoughts?

mgifford’s picture

Assigned: good_man » Unassigned

This still a concern in D8? Unassigned issue too.

kostyashupenko’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.64 KB

Re-rolled language_detection-1294946-59.patch for v-8.0.x-dev

Status: Needs review » Needs work

The last submitted patch, 64: language_detection-1294946-64.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

donquixote’s picture

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

casivaagustin’s picture

To implement a workaround in this issue I have implemented a link_alter hook, if there is a language option in the link variable I move the option to the query string so the session change the language and the editor will work.

function module_custom_link_alter(&$variables) {
  /**
   * This appends the lang code as query string in the url so the edit node can work with translations.
   */
  if ($variables['options']['language'] && $variables['url']->getRouteName() === 'entity.node.edit_form') {
    $variables['options']['query']['language'] = $variables['options']['language']->getId();
  }
}

Attached is a demo of the functionality working.

david.qdoscc’s picture

I am still experiencing this issue in Drupal 8.8.1

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

catch’s picture

octopus1’s picture

#74 I tested with drupal 8.8.4 and I think there is still a prob:
this is what i tested:
Having URL(prefixes) language detection and negociation method, I create a node with the default site language (en), when trying to acces the translated node in frensh (prefix fr-FR) I got an URL with no prefix, and it shows the default site language's node content. Is there a fiw to this prob ?