Audit of global language variable use

This is needed because currently content language inherits the value of interface language, so in core using the former or the latter makes no difference. But contrib modules can alter this, so we have to ensure all places use the right language type.

Comments are welcome.

Upgrading to critical as having parts of the drupal core using the wrong language type is a serious issue. OTOH this should not be a hard one to fix as it only involves switching some global variables around, but IMHO it's a must-fix.

I don't think this is critical, but it shouldn't be forgotten.
Perhaps adding the priority major tag is a good compromise?

Perhaps adding the priority major tag is a good compromise?

I'd like to have a committer feedback about this.

The thing which makes this more critical than normal is the comment language - since that could lead to inconsistent data, but I think it's going to be sufficiently rare that 'major' would be fine if we had that priority, but I imagine this will get patched and fixed before that priority is on Drupal.org anyway.

Looking at the table those changes all look fine, but there is no patch here, so back to active.

A couple of tricky ones are:

• drupal_render_cid_parts(), I'd like the hear moshe about it
• language direction, which IMO needs some thinking about

For drupal_render_cid_parts() I would use content language - if you need to do something tricky with user language you could specify that yourself via 'cid' instead of using the helper iirc.

language direction, no idea :(

drupal_render_cid_parts basically tries to recreate http://api.drupal.org/api/function/_block_get_cache_id/6. so, i would do whatever you would do for that code. hope that helps.

Removed my own comment as it was not relevant.

The patch performs the corrections suggested in #2.

The drupal_render_cid_parts() issue has been addressed by adding as cid parts all the configurable language types: this means that in core the behavior is exactly the same as before, but if a contrib module enables additional configurable language types (I'm thinking mainly about content language) they will be used as cid parts too. This choice should let us handle pages displaying content available in multiple languages which could have different cache entries for every combination of UI/content languages.

I left unchanged the language direction issue because I think UI language is the correct choice in the scopes it is used (i.e. page level). In #780508: Set language by content language for LTR/RTL display we can try to introduce a content-level scope to achieve separate language directions for page-level and content-level scopes (i.e. what's sketched in http://drupal.org/files/issues/rtl.png).

I forgot a comment.

Again wrong patch, sorry :(

Agreed on deferring language direction to #780508: Set language by content language for LTR/RTL display. The page/document must use UI language, inner elements may define language direction based on content language; specifying language itself additionally helps crawling machines/spiders.

+++ includes/common.inc	29 Jun 2010 22:16:48 -0000
@@ -5321,9 +5321,12 @@ function drupal_render_cid_parts($granul
+  // If Locale is enabled but we have only one language we do not need it as cid
+  // part.
+  if (drupal_multilingual()) {
+    foreach (language_types_configurable() as $language_type) {
+      $cid_parts[] = $GLOBALS[$language_type]->language;
+    }
   }

Very nice - dynamically using zero to both is exactly what I wanted to suggest. This should support any, from simple to advanced, language environments.

If I did not overlook something, then this patch contains all required changes already? Or are we still missing something? If not, RTBC?

44 critical left. Go review some!

@sun:

Thanks for reviewing! The patch in #14 should address all the pending issues except language direction.
Feel free to RTBC if you don't have any remark.

Related issue #855380: $language_url should be used to lookup the current path alias.

Committed to CVS HEAD. Thanks.

Tag update