I don't know if this is implemented in Drupal 7, but in Drupal 6 there is no HTTP-Header Content-Language which indicates the language of the current viewed page to the client. The HTTP-Header isn't really from interest to the user per se but it would help Browsers, Robots and all kind of other automated clients to determine faster which language is used on the page.

I implemented it in drupal_init_language and it's working perfectly:

/**
 *  Choose a language for the current page, based on site and user preferences.
 */
function drupal_init_language() {
  global $language, $user;

  // Ensure the language is correctly returned, even without multilanguage support.
  // Useful for eg. XML/HTML 'lang' attributes.
  if (variable_get('language_count', 1) == 1) {
    $language = language_default();
  }
  else {
    include_once './includes/language.inc';
    $language = language_initialize();
  }

  // Send appropriate HTTP-Header for browsers and robots (Google).
  header('Content-Language: '.$language->language);
}

I assigned it to Drupal 6.x-dev, but it would be something from interest for every new Drupal version. For a perfect i18n support things like that are important in my opinion.

Kindest regards, Richard

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

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

I think this is a great idea, however, in Drupal 7 there are multiple languages that can be used for the page. The interface and content can be different. So they still need to figure out between each other the right language to output there. I'd argue probably the interface language is the right one to include here, and then the content items should be marked up with language codes proper. That part is discussed at #1164926: Nodes need to have languages specified separately for accessibility.

mgifford’s picture

Gabor, this sounds like the right approach to me.

Any idea why drupal_init_language() was dropped in D7? Should it be added back in with D8?

The language info certainly doesn't seem to be sent to the browser.

Date: Wed, 15 Jun 2011 17:48:15 GMT
Server: Apache/2.2.9 (Debian) PHP/5.2.6-1+lenny10 with Suhosin-Patch mod_ssl/2.2.9 OpenSSL/0.9.8g
X-Powered-By: PHP/5.2.6-1+lenny10
Expires: Sun, 19 Nov 1978 05:00:00 GMT
Last-Modified: Wed, 15 Jun 2011 17:48:15 +0000
Cache-Control: public, max-age=0
Etag: "1308160095-0"
Link: ; rel="shortlink",; rel="canonical"
X-Generator: Drupal 7 (http://drupal.org)
Vary: Cookie,Accept-Encoding
Content-Encoding: gzip
Content-Length: 7700
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
Content-Type: text/html; charset=utf-8

200 OK

plach’s picture

The approach in #1 looks reasonable to me too. Can we have a patch?

@mgifford:

In d7 we have drupal_language_initialize() :)

http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal...

mgifford’s picture

Status: Active » Needs review
FileSize
681 bytes

@platch - How do we get it when you go to this page:
http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal...

There is some link that indicates that the function's name was changed in 7 and is now
http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal...

@Fleshgrinder - I don't know how many people actually use the HTTP-Header Content-Language - I couldn't find an example in Google, Flickr or the w3c. Who is using this? I don't doubt that it would be faster, but I don't know who is actually looking for it either. I'd like to know if there's a movement behind adopting this and what devices are using it.

That being said, here's a D8 patch.

Gábor Hojtsy’s picture

Status: Needs review » Needs work

Code style is not 100%, D7+ uses a space after and before concatenation. Otherwise looks good.

plach’s picture

Status: Needs work » Needs review

@mgifford:

It should have gone into: http://drupal.org/update/modules/6/7

I added a comment on the D6 API page.

mgifford’s picture

@Gabor - Forgot about the spacing, thanks! And to confirm, this will insert the interface language into the header and not the content's language which is the agreed best practice & consistent with what we do in the meta tags.

@plach - Thanks!

attiks’s picture

FileSize
680 bytes

small cosmetic change, remove empty line before closing }

plach’s picture

Gábor Hojtsy’s picture

Title: Content-Language HTTP-Header » Add the Content-Language HTTP header to the generated page
Issue tags: +D8MI

Cleaner title.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Actually look good to land IMHO. Very simple and useful.

webchick’s picture

Is there a reason we shouldn't backport to D7 as well?

Gábor Hojtsy’s picture

Issue tags: +Needs backport to D7

I think it good for D7 too. This is information we already put on the <html lang="" and xml:lang=""> attributes, but doing it over HTTP makes us even more flexible/compatible.

attiks’s picture

Patch applies cleanly to D7

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Sounds good!

Committed and pushed to 8.x and 7.x. Thanks!

moshe weitzman’s picture

Category: feature » bug
Status: Fixed » Active

Is there a reason this does not use drupal_add_http_header()?

moshe weitzman’s picture

Furthermore, this warns for all non http clients like drush scripts and drush.sh scripts. Please wrap like so

if (!drupal_is_cli()) {
  drupal_add_http_header()
}
Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
418 bytes

On #16: here is a quick patch.

On #17: I looked some random samples of the 23 places where drupal_add_http_header() is called and none of the have it wrapped like this. Is this a wider systemic problem or why would it only appear here? http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal...

Status: Needs review » Needs work

The last submitted patch, language-header.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
415 bytes

Should help if I use the API proper.

Status: Needs review » Needs work

The last submitted patch, language-header.patch, failed testing.

attiks’s picture

Status: Needs work » Needs review
FileSize
419 bytes

fixed typo

attiks’s picture

FileSize
494 bytes

Patch for moshe

plach’s picture

I share Gabor's perplexity: if non-HTTP clients should not get HTTP headers, why warnings are not throwed elsewhere? And if they do, why don't we simply move the test inside the drupal_add_http_header() function?

moshe weitzman’s picture

Status: Needs review » Needs work

@gabor - the other standard header() calls only happen inside of drupal_deliver_html_page(). This call should also be moved there. That would resolve the CLI issue that I mentioned.

attiks’s picture

Status: Needs work » Needs review
FileSize
1.06 KB

As explained in #25, new patch attached

attiks’s picture

FileSize
1.02 KB

cleaner patch, removed spaces

attiks’s picture

One remark: the header isn't added during install, but i guess it isn't that much of a problem?

Gábor Hojtsy’s picture

Status: Needs review » Needs work

@attiks: you also need to roll back the global $language from the first hunk then.

attiks’s picture

Status: Needs work » Needs review
FileSize
1.28 KB

as requested

Gábor Hojtsy’s picture

Looks complete. What do you think Moshe?

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

plach’s picture

I guess we need a side issue providing some test coverage for non-HTTP clients.

chx’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Looks like a bugfix to me and as such needs tests.

attiks’s picture

@chx, I don't mind adding a test, but what do you want to test, the presence of the header inside the response? Or are you talking about tests for the non-http clients?

Fleshgrinder’s picture

Sorry for the extremely late reply. Wasn't around for quite a while 'cause I was busy with a lot of other projects.

I also don't know of any special use cases of the Content-Language attribute. Like an other user stated before, it's mainly to be more flexible. The W3C encourages people to use the HTTP header field or the lang attribute on the html tag. By using both Drupal is prepared for any use case. [1]

Personally I think this can come in handy when dealing with search engines and this is the reason why I started to use it. Think of the following scenario. The bot indexes a new URL and saves it to the "crawl" repository and needs to get some basic data. For sure the bot will not download the whole page, but by just doing a HEAD request the bot can get all those basic information. If there is a standard compliant Content-Language field the bot directly knows which language is used within the page.

I know that this is an invented use case, but it's possible. Personally I think that there are for sure other use cases where this thing comes in handy. For instance for content negotiation [2] related things.

chx’s picture

Yes: create a test module with a different delivery callback and assert that Drupal added no http headers. Going to be tricky because the webserver also adds headers, right? Maybe try requesting an HTML page with drupalGet, store the name of the headers and ensure that a non-html delivery callback does not add more.

idflood’s picture

This issue breaks in some way drush (site-install command), see #1314392: drush site-install on Drupal 7.9 fails with a fatal error "Call to undefined function cache_get()"
I would love to help providing tests but I'm not comfortable with those and this one looks a bit harder to do. Any idea of similar tests which could be used as example?

attiks’s picture

@idflood you get the headers while using drush, as far as I can tell this should not happen, but true we need some kind of test for this.

moshe weitzman’s picture

Status: Needs work » Reviewed & tested by the community

Now this bug is in the D7 Drupal release and breaking Drush. We should be reverting features when we immediately notice that they introduce a bug. Please revert this feature or commit the fix in #30. Both are safe and will not break any tests. To revert, run:

git revert 07932805276fdaec77460e6aac330d11768d555c
git push
attiks’s picture

Status: Reviewed & tested by the community » Needs work

Patch in #30 is applied, see f429c2716ee22754e3fcccc91ad1818b9fda3785

Edit: This is the D7 one other the D8

moshe weitzman’s picture

@attiks - that's a prior patch in this issue. #30 moves the header call, which is a fix to the prior commit. so once again, please apply #30 or revert as per #40

attiks’s picture

Status: Needs work » Reviewed & tested by the community

clarified on irc and by actually looking at the code, please apply #30

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Crap, sorry. :( This wasn't tagged "Release blocker," nor was it critical, so I missed it in my final commits prior to 7.9.

However, I'm super confused because I didn't get that error locally when I drush sied 7.9 (that's part of the standard routine I do to check a release). Does this only happen in certain versions of Drush or am I just lucky?

[Edit: Yes, it occurs only on Drush 5.x, not Drush 4.x.]

Anyway, committed/pushed #30 to 8.x and 7.x. Leaving needs work for tests.

e2thex’s picture

This is a modification of #30 for use with drush make (removing the a/ and b/'s).

Note That I do not think this pass the test because of these change so this is only for use by those of use using drush make.

e2thex’s picture

My apologizes that patch was not converted correctly

This is a modification of #30 for use with drush make (removing the a/ and b/'s).

Note That I do not think this pass the test because of these change so this is only for use by those of use using drush make.

andypost’s picture

Status: Needs work » Needs review

Just to test #46

Status: Needs review » Needs work

The last submitted patch, i673020_deliver_1-make.patch, failed testing.

good_man’s picture

Status: Needs work » Needs review
FileSize
1.96 KB

Small test, anything more to add?

Gábor Hojtsy’s picture

Status: Needs review » Needs work
Issue tags: -Needs tests

Looks generally good. Here are some comments:

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -933,7 +933,8 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
+    $this->language = 'en';

Why does this need to be set?

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1030,6 +1031,36 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
+    // Add a new language and optionally set it as default.

Comment does not reflect what is happening right below it.

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1030,6 +1031,36 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
+    $this->assertEqual($request->headers['content-language'], 'en', t('content-language HTTP header is English.'));

The message can start with 'Content-Language', that is the HTTP name of the header no?

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1030,6 +1031,36 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
+    // Set French as a default language
+    $language = (object) array(
+      'language' => 'fr',
+      'name' => 'French',
+      'default' => 'fr',
+    );
+    locale_language_save($language);

The 'language' key changed to be 'langcode' in D8. The 'default' property should be TRUE if we want to make it the default. locale_language_save() is now language_save().

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1030,6 +1031,36 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
+    $this->assertEqual($request->headers['content-language'], 'fr', t('content-language HTTP header is French.'));

Same case fix as above.

-25 days to next Drupal core point release.

Gábor Hojtsy’s picture

Issue tags: +sprint

@good_man: can you help move this forward? I think I have some clear comments here. Thanks a lot!

Gábor Hojtsy’s picture

Tagging for base language system

rvilar’s picture

Assigned: Unassigned » rvilar

I'll work on this!

Zoltán Balogh’s picture

Status: Needs work » Needs review
FileSize
1.9 KB

A new version is attached.

Status: Needs review » Needs work

The last submitted patch, 673020_54_test_2.patch, failed testing.

Zoltán Balogh’s picture

Status: Needs work » Needs review
FileSize
1.85 KB

Status: Needs review » Needs work

The last submitted patch, 673020_56_test_3.patch, failed testing.

Zoltán Balogh’s picture

Assigned: rvilar » Zoltán Balogh
Status: Needs work » Needs review
FileSize
1.58 KB

One more try.

Gábor Hojtsy’s picture

Status: Needs review » Needs work
+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1033,6 +1033,25 @@ class CommonDrupalHTTPRequestTestCase extends DrupalWebTestCase {
+   * Test HTTP headers of Drupal request.

Let's say "Tests Content-language headers generated by Drupal." (3rd person verb to start; also only testing the language header, so better be specific).

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1033,6 +1033,25 @@ class CommonDrupalHTTPRequestTestCase extends DrupalWebTestCase {
+    // Set French as the default language

It is not default anymore, so let's just say "Add French language." or something along those lines.

+++ b/core/modules/simpletest/tests/common.testundefined
@@ -1033,6 +1033,25 @@ class CommonDrupalHTTPRequestTestCase extends DrupalWebTestCase {
+
+    $request = drupal_http_request(url('<front>', array('absolute' => TRUE, 'language' => $language)));
+    $this->assertEqual($request->headers['content-language'], 'fr', t('Content-Language HTTP header is French.'));

Let's add a line of comment here like above. "Request front page in French and check for matching Content-language" or somesuch.

kalman.hosszu’s picture

Status: Needs work » Needs review
FileSize
1.66 KB

Patch updated based on Gábor's comments.

kalman.hosszu’s picture

The test run successfully, so I think we should change the status to RTBC.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

I agree this looks good now. We debated a bit as to whether put this in with locale or language module, but it looks fine with the general http header tests AFAIS.

Dries’s picture

#60: 673020-60.patch queued for re-testing.

Dries’s picture

Gábor Hojtsy’s picture

Still passes. This modifies a common test for HTTP headers so no interference with other language changes. Still RTBC :)

Gábor Hojtsy’s picture

Title: Add the Content-Language HTTP header to the generated page » Tests for adding the Content-Language HTTP header to the generated page

Also retitle to make it clear we are only doing tests here, the feature is in already.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

I thought it was going to be ok to commit, but the tests do pass langcodes in the URL so I wan't 100% sure. Better safe than sorry. Glad to tests still passed. Committed the tests to 8.x and marking this 'fixed'. Another D8MI patch bites the dusts. I'm happy with all the D8MI progress.

Gábor Hojtsy’s picture

Version: 8.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)

As per @webchick in #44, tests need to go to D7 too. So reopening as a backport. Might apply 1-1 to Drupal 7.

Zoltán Balogh’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.63 KB

Ported to 7.x.

Zoltán Balogh’s picture

Oh, it will be bad, there is not langcode in 7.x :)

Zoltán Balogh’s picture

FileSize
1.63 KB

Corrected

Status: Needs review » Needs work

The last submitted patch, 673020-72.patch, failed testing.

Zoltán Balogh’s picture

Assigned: Zoltán Balogh » Unassigned
Zoltán Balogh’s picture

Status: Needs work » Needs review
FileSize
1.54 KB

Status: Needs review » Needs work

The last submitted patch, 673020-75.patch, failed testing.

Zoltán Balogh’s picture

Status: Needs work » Needs review
FileSize
1.54 KB

Status: Needs review » Needs work

The last submitted patch, 673020-77.patch, failed testing.

Zoltán Balogh’s picture

Assigned: Unassigned » Zoltán Balogh
Gábor Hojtsy’s picture

@Zoltán: the patch in #75 was better, you need to pass on a 'full' language object to url(). So after locale_add_language(), clear the language_list cache and get the list of languages to have the language object. Something like:

    // Add French language.
    locale_add_language('fr', 'French');
    drupal_static_reset('language_list');
    $languages = language_list();

    // Request front page in French and check for matching Content-language.
    $request = drupal_http_request(url('<front>', array('absolute' => TRUE, 'language' => $languages['fr'])));

(Not tested).

Zoltán Balogh’s picture

Status: Needs work » Needs review
FileSize
1.58 KB

It was ok in the local environment.

Zoltán Balogh’s picture

@Gábor, locale_add_language('fr', 'French'); causes a PDOException in D7:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'native' cannot be null:

I switched to German, to avoid the use of special characters in the native name.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

@Zoltán: right, it needs more arguments in Drupal 7. Making the new language the default works too like in your last patch. Backport looks good.

xjm’s picture

+++ b/modules/simpletest/tests/common.testundefined
@@ -1035,6 +1035,22 @@ class DrupalHTTPRequestTestCase extends DrupalWebTestCase {
+    // Add German language and set as default.
+    locale_add_language('de', 'German', 'Deutsch', LANGUAGE_LTR, '', '', TRUE, TRUE);
+    ¶

Trailing whitespace. :)

Also, the assertion messages need not be translated, but that already went in 8.x, so we shouldn't fuss with the backport.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work

Rrrright :)

Zoltán Balogh’s picture

Status: Needs work » Needs review
FileSize
1.57 KB

Thanks @xjm, I corrected. 7.x in first round.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good again. Lacks line ending whitespace now :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Great!

Committed and pushed to 7.x. Thanks!

Gábor Hojtsy’s picture

Issue tags: -Needs backport to D7, -sprint

Done with it being on focus then, thanks.

Zoltán Balogh’s picture

No need to remove t () in 8.x from the assertion messages?

Gábor Hojtsy’s picture

@Zoltán: if you look at modules/simpletest/tests/common.test in D8, you'll see it is chock full of t() on assertions. Unifying things like these would indeed be great, the coding standards for tests don't seem to have guidelines either way: [#325974]. Anyway, I think cleaning that up would be its own issue.

xjm’s picture

http://drupal.org/simpletest-tutorial-drupal7#t
but also note that #500866: [META] remove t() from assert message is still NW, so while it's nice to fix that before patches go in, I think webchick will hurt me if I slow a patch over it. :)

Status: Fixed » Closed (fixed)
Issue tags: -D8MI, -language-base

Automatically closed -- issue fixed for 2 weeks with no activity.