$language_url is only set if language is detected from the URL

Only cross-linking to g.d.o.: URL language vs. user interface language: D7 $language_url global

@mfb:

Shouldn't $language_url be set regardless of the language detection method?

No, otherwise there would be no point in having two distinct variables: the reason is that they may actually be different.

$language_url is set in drupal_language_initialize() and language_initialize(). A module could presumably implement hook language_init() to set $language_url = $language during bootstrap.
In any case, I don't really understand the logic behind the user interface language detecting browser language while URL language ignores it.

See http://drupal.org/update/modules/6/7#language_negotiation for a detailed explanation. You may also want to give a look to this (closed) related issue, that may help to shed some light on this one: #855380: $language_url should be used to lookup the current path alias.

Anyway the behavior you are reporting in the OP is wrong and we must fix it, I suspect your system is configured so that all languages have a non-empy language prefix, right?

If the default language had an empty prefix you would get a page with URLs with an empty prefix, and every page would behave as the viewed one, e.g.: you get a Spanish interface and URLs are unprefixed. If I understood the bug report well, here we are viewing a page with an unprefixed URL, say /node, but all the enabled languages have a prefix defined. In this case (and only in this case) I agree that if we don't find a language prefix we must fall back to the page language.

I'll provide a patch myself. I have an idea on how to quick fix this.

Here is a patch for the bot. We still needs tests.

The attached patch fixes the typos, provides an update function that refreshes the language negotiation settings and adds test coverage. We should be ready to go now.

Tests pass on my box

#9: language-956256-9.patch queued for re-testing.

Let's try this.

There was the usual problem with clean urls. This one should pass.

There was also an issue with the base path. Hopefullly this one should be ok, provided that the patch is in a correct format (I had to manually edit the previous one, since I cannot login into cvs.drupal.org atm).

Rerolled #16 now that cvs.drupal.org is back online.

whew

+++ includes/locale.inc	2 Nov 2010 14:35:03 -0000
@@ -23,6 +23,12 @@ define('LOCALE_LANGUAGE_NEGOTIATION_BROW
+ * The language is determined using an already detected one if no URL language
+ * is available.
...
+define('LOCALE_LANGUAGE_NEGOTIATION_URL_FALLBACK', 'locale-url-fallback');

Had to read this twice to understand what it tries to tell. Can we move the if clause to the beginning?

+++ includes/locale.inc	2 Nov 2010 14:35:03 -0000
@@ -78,6 +84,28 @@ function locale_language_from_interface(
+ * Identifies the language to be assigned to URLs when none is detected.

The summary states that a language is assigned *to* a URL, but the actual function name states that a language is determined *from* a URL. Which of both?

+++ includes/locale.inc	2 Nov 2010 14:35:03 -0000
@@ -78,6 +84,28 @@ function locale_language_from_interface(
+ * If no URL language can be detected and the default language is configured to
+ * always provide language information via URL prefix or domain, we need to fall
+ * back to the most suitable language, which is an already detected language.

1) "we" should not be contained here.

2) Without reading the rest of this patch, it's not clear to me,

a) from where locale_language_from_url_fallback() is invoked

b) whether a special configuration is required to make the language system invoke this function

c) how and where the assumption that other languages have been already detected is accounted for.

+++ includes/locale.inc	2 Nov 2010 14:35:03 -0000
@@ -78,6 +84,28 @@ function locale_language_from_interface(
+ * @param $languages
+ *   Optional. An array of valid language objects. Defaults to NULL.
+ * @param $language_type
+ *   Optional. The language type to fall back to. Defaults to the interface
+ *   language.

1) "Optional." should be "(optional)" (lowercase, parenthesis). See http://drupal.org/node/1354 for details.

2) $language doesn't seem to be used in this function. It's not clear to me what happens if I pass a certain $language, or what the default/special value of NULL means.

+++ includes/locale.inc	2 Nov 2010 14:35:03 -0000
@@ -78,6 +84,28 @@ function locale_language_from_interface(
+  $prefix = variable_get('locale_language_negotiation_url_part', LOCALE_LANGUAGE_NEGOTIATION_URL_PREFIX) == LOCALE_LANGUAGE_NEGOTIATION_URL_PREFIX;
+  return ($prefix && empty($default->prefix)) || (!$prefix && empty($default->domain)) ? FALSE : $GLOBALS[$language_type]->language;

1) Ideally, there should be parenthesis around the entire comparison for $prefix; i.e.:

$prefix = (variable_get() == LANGUAGE);

2) The return line is too condensed and should be written with a proper if/else construct (which additionally allows to write inline comments above the if/else conditions to explain why a certain check means foobar.

+++ modules/locale/locale.install	2 Nov 2010 14:35:03 -0000
@@ -113,6 +113,18 @@ function locale_update_7001() {
+function locale_update_7002() {
...
+  return array();

return can be removed.

+++ modules/locale/locale.install	2 Nov 2010 14:35:03 -0000
@@ -113,6 +113,18 @@ function locale_update_7001() {
+  $language_types_info = language_types_info();
+  $info = $language_types_info[LANGUAGE_TYPE_URL];
+  if (isset($info['fixed'])) {
+    language_negotiation_set(LANGUAGE_TYPE_URL, array_flip($info['fixed']));

1) $language_types_info is $language_types elsewhere, I think.

2) $info is very generic, why not $url_language ?

+++ modules/locale/locale.module	2 Nov 2010 14:35:03 -0000
@@ -582,6 +582,15 @@ function locale_language_negotiation_inf
     'description' => t('Use the detected interface language.'),
...
+    'description' => t('Return a suitable language for URLs if none is detected.'),

I guess that other descriptions start with "Use", which makes more sense from a UX perspective than a technical "Return", so we should re-phrase this description to also start with "Use".

+++ modules/locale/locale.module	2 Nov 2010 14:35:03 -0000
@@ -582,6 +582,15 @@ function locale_language_negotiation_inf
+    'name' => t('URL Fallback'),

"Fallback" should be lowercase, I think.

+++ modules/locale/locale.test	2 Nov 2010 14:35:03 -0000
@@ -1892,6 +1891,55 @@ class LocaleUILanguageNegotiationTest ex
+  function testUrlLanguageFallback() {
+    drupal_static_reset('language_list');

This initial static reset is either superfluous or needs an inline comment for why it is required. (I bet it's superfluous.)

+++ modules/locale/locale.test	2 Nov 2010 14:35:03 -0000
@@ -1892,6 +1891,55 @@ class LocaleUILanguageNegotiationTest ex
+    $path = !empty($GLOBALS['conf']['clean_url']) ? $language_browser_fallback : "?q=$language_browser_fallback";
+    $args = array(':url' => base_path() . $path);
+    $xpath = '//div[@id="block-locale-language"]//a[@class="language-link active" and @href=:url]';
+    $fields = $this->xpath($xpath, $args);
...
+    $xpath = '//div[@id="site-name"]//a[@rel="home" and @href=:url]//span';
+    $fields = $this->xpath($xpath, $args);

These separated code lines look a bit weird. Can we write this like we do elsewhere?

$elements = $this->xpath('//div...', array(
  ':url' => base_path() . (!empty(...),
));

Powered by Dreditor.

Thanks sun! Everything should be fixed except:

1) $language_types_info is $language_types elsewhere, I think.

language_types_info() return an indexed array whose keys are language types, but values are again indexed arrays of data. The only other piece of core code invoking it uses $language_types_info.

2) $info is very generic, why not $url_language ?

Because $info holds the language negotiation info for URL language not a language value.

+++ includes/locale.inc	4 Nov 2010 10:26:34 -0000
@@ -211,6 +217,42 @@ function locale_language_from_url($langu
+ *   A valid language code on success, FALSE otherwise.

We always have a valid language code.

Powered by Dreditor.

Discussed with sun a far better PHP doc for locale_language_url_fallback().

Thanks! We can with this patch and documentation for now. Ideally, we'll revisit the entire high-level and low-level language system documentation in a separate (non-major) issue.

This needs to be committed before rc-1, since we are introducing a couple of new strings (although in core they are never displayed).

Committed to CVS HEAD. Thanks.

Thanks Dries!

Quick followup: #965130: locale_update_7002() assumes that multiple languages are enabled..