$language_url should be used to lookup the current path alias

That's not a quick fix.

+++ modules/path/path.test	15 Jul 2010 16:24:53 -0000
@@ -242,8 +242,8 @@ class PathLanguageTestCase extends Drupa
-    $this->drupalLogin($web_user);
...
+    $this->drupalLogin($this->user);

This should be $this->web_user. I actually thought that $this->user would be a backup of the local user running simpletest.

+++ modules/path/path.test	15 Jul 2010 16:24:53 -0000
@@ -268,26 +264,26 @@ class PathLanguageTestCase extends Drupa
+    $french_alias = $this->randomName();

@@ -306,6 +302,40 @@ class PathLanguageTestCase extends Drupa
+    // Check that the French alias works.
+    $this->drupalGet("fr/$french_alias");
...
+    // Disable URL language negotiation.
...
+    // Check that the English alias still works.
+    $this->drupalGet($english_alias);
...
+    // Check that the French alias is not available.
+    $this->drupalGet($french_alias);
+    $this->assertResponse(404, t('Alias for French translation is unavailable when URL language negotiation is disabled.'));

Please bear with me. I'm a crazy old man.

1) We are testing two different URLs for the same $french_alias here. First with an additional "fr/" prefix, afterwards without. Somehow, but I've to admit that I never really checked this, I assumed that, when storing language-specific aliases, then that entire alias would be used; without path prefix, as that prefix, like the name implies, belongs to the path, not the alias...? Am I horribly mistaken?

2) This test enables both user language and URl language negotiation, preferring user language over URL language. It's configuring the user's preferred language to French. Afterwards, it disables URL language negotiation. Keeping user language negotiation enabled. Lastly, it directly visits the English node page. -- I totally thought that one of the following would happen:

a) As the user still has a preference, user language negotiation is still enabled, and there is a French translation, he gets or is redirected to the French node.

b) Basically same as a), but the test should have visited a node listing page (such as the front page), where language negotiation actually happens, so the French translation (the user's preference) actually appears.

No? Am I nuts? :)

+++ modules/path/path.test	15 Jul 2010 16:24:53 -0000
@@ -306,6 +302,40 @@ class PathLanguageTestCase extends Drupa
+    // Confirm that the alias works even changing language negotiaion options.

1) Missing "when" after "even".

2) Typo in "negotiation".

Powered by Dreditor.

subscribe

Thanks for the thorough explanation!

+++ includes/path.inc	15 Jul 2010 18:40:31 -0000
@@ -71,7 +71,7 @@ function drupal_lookup_path($action, $pa
-  $path_language = $path_language ? $path_language : $language_content->language;
+  $path_language = $path_language ? $path_language : $language_url->language;

Given that this is a critical issue and contains quite some explanation and reasoning, it would be good to transfer the major information building blocks into a in-code comment, explaining why the fallback is $language_url and not something else, so as to prevent breaking this again in the future.

+++ modules/path/path.test	15 Jul 2010 19:06:41 -0000
@@ -306,6 +302,40 @@ class PathLanguageTestCase extends Drupa
+    // Check that the French alias is not available.
+    $this->drupalGet($french_alias);

Let's append your explanation to the comment:

"Check the unprefixed alias, because we disabled URL language negotiation above. In this situation, only English and language neutral aliases should keep working."

(although... "English"...? Did you mean the site's default language?)

Furthermore, let's additionally append:

"If URL language negotiation is disabled, then French will be the active UI and content language (the user's preference), and English the active URL language, as no prefix was found and we fall back to the site's default language. [NOTE: needs tweaking:] In this case, drupal_lookup_path() needs to use the URL language to check whether a URL alias is valid."

[TWEAK WITH THIS: if we pick the content language we have no way to check that the path alias is valid: there is no path alias for french matching the english alias, so we can't get the english node and we can't get its french translation. So we have to pick URL language.]

46 critical left. Go review some!

Awesome! Much much better now. With these, no one needs to read this issue to understand what's going on.

Also related to drupal_path_alias() bugs

#829968: drupal_lookup_path() documentation and return mismatch
#863318: Wrong sort order of aliases for different languages

This looks good and the code comments make it easy to grok. Great work. Committed to CVS HEAD.