Non-localized links in translation table...

Reroll against head.

Hmmz.. fixed the typo's.

+++ modules/translation/translation.pages.inc	21 Jun 2010 22:51:08 -0000
@@ -33,9 +33,9 @@ function translation_node_overview($node
+      $title = l($translation_node->title, 'node/' . $translation_node->nid, array('language' => $language->language));
       if (node_access('update', $translation_node)) {
-        $options[] = l(t('edit'), "node/$translation_node->nid/edit");
+        $options[] = l(t('edit'), "node/$translation_node->nid/edit", array('language' => $language->language));

l() needs a language object, not a language code.

Moreover we need a test ensuring we won't break this behavior in the future.

Powered by Dreditor.

Hmmz plach normally I would agree but look at current core code, line number 50:
http://drupalcode.org/viewvc/drupal/drupal/modules/translation/translati...

At the moment in the same function it already uses $language->language which I followed. Should all 3 instances be fixed to give the language object?

At line 50 the language code is used to format the query string not as an option for l(). See http://api.drupal.org/api/function/url/7 for details.

Anyway I tested the patch and it does not work without the fix I suggested.

Oh, that is true. I'l reroll!

It seems the patch is somehow corrupted. Can you post it again?

You need to reroll the patch from the drupal root.

However this does not work when adding a translation in a disabled language (see #356036: Allow creating nodes in disabled languages). You should use http://api.drupal.org/api/function/language_negotiation_get_switch_links/7 for each path to get the proper URL for each language.

It's pretty easy to reproduce: enable french and german, disable french and then try to add a translation for it (if you can't see a use case for this please read the issue above). As language negotiation works only with enabled languages /fr/node/x won't work. However I performed some tests and this turned to be another issue.

Edit: related issue: #926212: URLs rewritten with disabled languages.

@bforchhammer:

The patch looks good and works as intended. If you provide a simpletest I'm ready to RTBC it.

Other than the minor coding standard issues below this looks good to go :)

+++ modules/translation/translation.test
@@ -45,14 +52,23 @@ class TranslationTestCase extends DrupalWebTestCase {
+    $this->assertLinkByHref( $languages['es']->prefix . '/node/add/' . str_replace('_', '-', $node->type), 0, t('The "add translation" link for %language points to the localized path of the target language.', array('%language' => $languages['es']->name)));

The whitespace between the method parenthesis and the arumgent shouldn't be there.

+++ modules/translation/translation.test
@@ -45,14 +52,23 @@ class TranslationTestCase extends DrupalWebTestCase {
+    $this->assertLinkByHref( $languages['es']->prefix . '/node/' . $node_translation->nid . '/edit', 0, t('The "edit" link for the translation in %language points to the localized path of the translation language.', array('%language' => $languages['es']->name)));
+    $this->assertLinkByHref( $languages['es']->prefix . '/node/' . $node_translation->nid, 0, t('The "view" link for the translation in %language points to the localized path of the translation language.', array('%language' => $languages['es']->name)));

idem

Powered by Dreditor.

Nice!

@bforchhammer:

Sorry for changing status but #926212: URLs rewritten with disabled languages should be fixed before committing this, otherwise we would introduce a major core misbehavior.

Could you stay tuned over there? I'll try to provide a simple test ASAP. If you can test and review it too, this would speed up the commit of this issue. I'll RTBC this again once the other one is RTBC.

@Dries, @webchick:

Before committing this, be sure to commit #926212: URLs rewritten with disabled languages as this exposes the bug fixed over there.

After some additional testing I realized that the last patch is incomplete: if you enable the Session language detection method instead of the URL one the translation links are still unlocalized.

As I suggested in #21 we should use http://api.drupal.org/api/function/language_negotiation_get_switch_links/7 to obtain the proper localized links. We must also keep in mind that when no language detection method is enabled its result will be empty, but in this case we don't need localized links.

This is not a defect in url(): in the latest patch we are esplicitly providing an option to change the language, but this concerns only URL language, while to switch session language we need to provide a query parameter. But this would not take into account any other possible contrib language detection method. http://api.drupal.org/api/function/language_negotiation_get_switch_links/7 does by invoking all the defined language switch callback functions. Using it is not that hard after all.

I understand you perplexity: the reason is that with the new language negotiation API a URL can convey an arbitrary number of language types, so we can't simply rely on $options['language'].

Let's try this.

Tests should pass now.

#38: translation-284625-38.patch queued for re-testing.

#38: translation-284625-38.patch queued for re-testing.

#926212: URLs rewritten with disabled languages committed. Sent for a re-test.

However, I reeeeeeeeally am not a fan of introducing a "special" l() function here. Can we just include that code in translation_node_overview()?

And here it is :)

this should work

Thanks!

Committed to HEAD.