Browser language detection is skipped for anonymous users when page caching is turned on, with no warning to the site administrator

I noticed this at: https://api.drupal.org/api/drupal/includes%21locale.inc/function/locale_...

We perform browser accept-language parsing only if page cache is disabled, otherwise we would cache a user-specific preference.

So, by the statement "otherwise we would cache a user-specific preference", I understand the problem would be in the following scenario if we allowed :
Page cache: enabled
Language browser detection: Ignores page cache setting (i.e. browser language negotiation allowed with page cache turned "on")
Then,
User 1 browser negotiates a French language and he is produced the French version of the page. But since, the page cache is enabled, the french version of the page is cached.
At a later time, User 2 browser negotiates an English language. But since the page is already there in the cache (even though in French) and the page cache is given higher preference over the browser negotiated language, the user might be displayed the French version page.

So, I believe it needs to have a design change so that the cache should be able to store multiple version of the page for each of the language. So, the current design only seems to avoid the above problem till the time we have support for the different language versions of a page in the cache.

Please correct my understanding if I am wrong somewhere.

Cheers.

The main question is probably: Why should the result of the browser language detection not be cached within a single page request?

You are mis-reading the intent of the code. This code prevents language negotiation when the page cache is enabled. It gives the language negotiation methods the opportunity to say:

• "I don't care about the page cache" ('cache' not set or NULL)
• "I can only work with page cache disabled" ('cache' => 0)
• or "I can only work with page cache enabled" ('cache' => 1)

Browser-based language negotiation is incompatible with the page cache, at least in Drupal 7 (because the page cache doesn't take the language into account when caching or serving pages). So this code is working as intended.

The code in question doesn't *affect* the page cache, it *reacts* to it being enabled or not.

The browser language detection was probably made incompatible with the page cache to support the (fairly common) case of a default language with an empty prefix. Tracking down the issue that introduced this should not be too hard (just look at the git blame).

I can confirm that the problem still exists and it's a big problem for a customer page for us.

Furthermore I can confirm that the quickfix above works fine and from my point of view the conclusions of this thread are correct.

I'm having the same issue.

Occurs here too, in my contrib module: #2302975: Not working for anonymous user

Same problem here for our site that uses ip2country based language negotiation. The only problem, I think, would be the frontpage (which has cache url ""), as all other pages have different URLs for each translation. By using cache exclude (https://www.drupal.org/project/cacheexclude) and disabling cache for the frontpage, we can work around the problem. Luckily, our frontpage is not too heavy.

I can confirm this problem on several sites.
This problem is resolved with the quickfix.

Will this be addressed in Drupal 8?

This is fixed already in Drupal 8 because there the browser negotation automatically disables page cache if the browser negotation is triggered. This was identified and added in #2368987: Move internal page caching to a module to avoid relying on config get on runtime. It's a by far better solution, because page cache can be enabled and still works on all other pages *and* when a language prefix is provided.

Additionally, #2430335: Browser language detection is not cache aware will improve it so that it will redirect instead of returning an uncacheable page. The same behavior is possible in 7.x in combination with the globalredirect module.

Attaching two patches. The first just removes the cache flag from the browser negotation and also adds the flag to disable page caching. The second one removes that cache checking completely. This might not be backportable in 7.x, but I wanted to upload a patch for that anyway.

Note: When using/testing the non-remove patch, you must re-save the negotation settings. The information is read from the variable cache, and that is only changed when you save that form.

That's exactly the way to go. I've fixed it the same way in the IP Language Negotiation module: http://cgit.drupalcode.org/ip_language_negotiation/commit/?id=0f4568f

Didn't test the patch, but the code looks good to me.

RTBC, looks great to me!

Thanks, probably makes sense to add some tests, or update the existing tests to have page cache enabled, though?

@Fabianx: which one of the two patches did you mean to RTBC? I think changing the API in the way with the full remove patch may not be possible anymore in D7...

Yes, I meant only the #13 normal patch with my RTBC.

The reason is that any contrib/ module can do the same, so we can deprecate / strongly recommend against using 'cache' key for now, but still keep BC.

Ok, here are tests. It's not passing locally, I have some exceptions from the logout, will need to restructure the test if that happens on testbot too.

Also had to make one small change and always call drupal_page_is_cacheable(FALSE); even if there's no accept-langcode header. That shouldn't make a big difference to real browser, but otherwise, something like curl can "pollute" the page cache.

RTBC, looks great now!

Patch in #20 works for me, thank you

I'm getting these kind of errors when going to the translation interface page (admin/config/regional/translate/translate):

Notice: Undefined index: views in _locale_translate_seek() (line 1936 of includes/locale.inc).

I'm using the patch from #20. Could this be related? (Clearing the cache doesn't make it go away)

This looks like a good option, but I don't see how we can just change the default behavior like that? Silently disabling the page cache on many pages of the site might not be what they want at all, and could bring the site to its knees.

Shouldn't this be a configuration option? Many of the other language detection methods have configuration pages, and this could have one too ("Choose how browser detection is handled for anonymous users when page caching is turned on" => "Skip browser detection" or "Disable page caching for pages that requires browser detection", something like that + plus maybe a note about what the current sitewide page cache setting is).

I'm also changing the title here. It looks like the current functionality is behaving as intended/documented, just that it's inflexible and there's no warning to the administrator about what is going on.

By the way, @knalstaaf, looking at the patch it does not seem likely that your errors are related.

I'm attaching a ready-to-use fix-module for Drupal 7 in case anyone needs to use it

I guess the patch from #20 should not be hidden as it is the more definite solution in core (which I can confirm it works).

I'm also changing the title here. It looks like the current functionality is behaving as intended/documented, just that it's inflexible and there's no warning to the administrator about what is going on

You're saying that disabling page cache on those requests is a problematic fix that could bring sites down and at the same time that this is working fine as long as you disable page cache completely. What now? :)

I'm not sure I see the point in a configuration option. That's essentially deciding between it working or not working? Why would I want to enable that feature but then have it not working for anon users? I think we can just as well show an information that enabling this will disable page cache on pages where this negotiation method is applied, if you don't want that, don't enable it?

Why would I want to enable that feature but then have it not working for anon users?

I think it's definitely possible that a site would want this to be enabled as an enhancement for authenticated users only (i.e., the current behavior). That would mean that if you have an account, the site can guess the correct language for you based on your browser settings, but if not you simply use some other method (e.g. choosing a language manually via the language switcher block).

On a high traffic site I would probably prefer that to disabling the page cache. (And I especially wouldn't want a Drupal point release to decide to disable the page cache for me.)

I needed a browser detection method that would work for both anonymous AND logged in users without impacting the site performance by disabling the cache but none of the proposed fixes really made much sense to me when the most logical seemed to be a simple re-direct. For my use-case, the following in a custom module worked better than anything listed here and I'm putting it here for my own reference for when I'll need it again. The only thing that really needs fleshing out is the excludes portion. I've already added ajax and drush, but I'm sure there will be others that need to be added in time.

The way this works is that it only allows detection once per SESSION. That is, it detects language from the browser, but will only do so once. The reason is because if it always does detection, then the default language is never selectable from a language dropdown from a browser thats using the non-default language. The quickest way around that is to simply only do detection once. The other thing that this avoids is all the cache non-sense that this issue has had a problem with. Using this method, it doesnt matter if the page is cached or not because it's always using the CORRECT language for the user. So cached or not, it wont matter.

Anyway, place the following in a custom module, and forget this problem ever existed for drupal core :) If you can assist with this, please improve the "Excludes" section. Everything else should be just fine.

function YOURMODULE_boot()
{
   YOURMODULE_detect_language();
}

function YOURMODULE_detect_language()
{
   //No need for this function when drush is being used.
   if (drupal_is_cli())
   {
      return;
   }

   //Do not run if this function has already been called for this user's session already.  We really only need to call this once per session.
   //If this code is missing, then the end result is that the default language cannot be selected by a browser that is using a different language.
   //This code prevents that logic loop by only allowing detection to take place once per session.
   if (!empty($_COOKIE['drupal_locale_browser_detected']))
   {
      return;
   }

   //TODO:  Place some URL excludes here such as ajax and others.  Please help fill out this part of the code with urls that should be excluded.
   if (!empty($_SERVER['X-Requested-With']) || (!empty($_SERVER['X-Requested-With']) && $_SERVER['X-Requested-With'] == 'XMLHttpRequest'))
   {
      return;
   }

   require_once("modules/locale/locale.module");
   require_once("includes/locale.inc");
   require_once("includes/menu.inc");
   require_once("modules/user/user.module");
   require_once("includes/common.inc");
   require_once("includes/path.inc");

   $languages = locale_language_list('name');
   $browser_language = locale_language_from_browser(language_list());

   //Dave described a problem with cache and Session,  so I've modified it to use Cookie instead.
   //Set cookie with no expiration date so that it expires as soon as the browser closes.
   setcookie('drupal_locale_browser_detected', TRUE);
   if ($_SERVER['REQUEST_URI'] == base_path() && array_key_exists($browser_language, $languages) && $browser_language != 'en')
   {
      drupal_goto("/{$browser_language}");
      exit;
   }

   $node = menu_get_object();

   //Only check browser if we are using the default language OR the current page isnt a node which isnt translatable anyway.
   if (empty($node->nid) || ($node->language != 'en') || $browser_language == 'en')
   {
      return;
   }

   $node_translations = translation_node_get_translations($node->nid);

   if (array_key_exists($browser_language, $languages) && $node->language != $browser_language && array_key_exists($browser_language, $node_translations))
   {
      $path = "/{$browser_language}/". drupal_get_path_alias('node/'.$node_translations[$browser_language]->nid, $browser_language);
      drupal_goto($path);
      exit;
   }
}

@crystaldawn, it looks like your code sets a $_SESSION value for every single visitor to the site and then never removes it. Won't that have the effect of disabling the page cache completely?

(See for example drupal_session_initialize() and drupal_page_set_cache().)

That would be worse for performance than the earlier patch in this issue, which as I recall only disabled the page cache sometimes.

You're right that doing this via a redirect is a promising feature though. See #2430335: Browser language detection is not cache aware (which was linked above) for that.

What? No. Why would setting a temporary session var destroy caching? That makes no sense at all. What would cause it to do that? Even so, if it did, then a short expiring cookie would do the exact same thing or local storage. Please explain. How adding a session Car disables caching. And there is no need to clear the var because that will happen when an anonymous user closes their browser or a logged in user logs out. It's likely best to avoid having to set any sort of timer on it or it could trigger right in the middle of a user's experience.

If you try it out and examine the request headers, e.g. with Firebug, I think you'll see that users don't get delivered cached pages anymore, the next time their request hits the server after adding your code. I tried it with a stripped-down version of the code in hook_init(), at least, and that's what I observed. My code was basically just this:

if (empty($_SESSION['test'])) {
$_SESSION['test'] = 'test';
}

As for the reason, see the code I linked to. Cached pages are the same for everyone, but $_SESSION persists between requests and is specific to a user. So if the user has active data in $_SESSION they need to bypass the cache and do a full Drupal page request so their specific $_SESSION value can be used. At least, that's what I assume the logic behind all that is :)

(I guess you could imagine an optimization to that whereby Drupal could have a second caching layer that occurred later in the bootstrap - after the session is loaded - and that used the $_SESSION value as part of the cache key so that users with the exact same $_SESSION variable could still share a cache, but that would be pretty complicated and would have diminishing returns.)

I guess I am confused then because while it's true that SESSION is not cached, that should not effect how the page itself is actually loaded on the server's end. I'm going to actually try this because I've seen absolutely no performance degradation at all and its VERY noticeable with this particular site if caching is off. It's certainly acting like it's on. My planned test is going to be a plain jane drupal with a page that loops mysql read/write queries over and over and over 10000 times which will cause it to be VERY slow. Then enable caching which should stop that. Then I'm going to put code right before the actual cache loading SQL query in drupal core and see if it's ever hit and if it is, I should see the page load quickly. If it's missed completely, then I'll have to say you're right (but not sure what logic would ever cause that to be the case, I cant think of a useful usecase for such a scenario offhand). However another possibility is that it still loads the cache but then doesnt bother to use it which would again result in a slow page. In which case I'd consider that a pretty major flaw. I dont expect that to happen either.

I am aware that being logged in as a user disables caching and there are very good reasons for that. But for anonymous users..... this should not be the case at all and using session should not disable drupal's cache. Or it shouldnt anyway, that would be kinda stupid.

Well that didnt take long. I'm happy to report that I had the exact behavior I thought I would have. The view loaded perfectly for anonymous users and it was slow as molasses for logged in users (since its not cached which is a known behavior for logged in users in drupal anyway).

I'm not sure why you think SESSION causes caching to be disabled but my own tests dont exactly prove your point (which I'm relieved about lol).

Through out the test, the session variable was always available. I closed the tab (without closing browser) and of course it still had the session variable, which is what I would want. When I closed the browser, it again had to create the session variable. All of these were variable changes were seen without ever hindering the drupal cache (in this specific case, it was a custom view that ran 10000 read/write queries in an attempt to make the page super slow on purpose).

Is the behavior you are talking about documented anywhere? I couldnt find anything really.

Also to note, Cache-Control:public, max-age=86400 was still passed to the browser for anonymous users while logged in users got Cache-Control:no-cache, must-revalidate. No amount of adding or removing SESSION var in hook_init of a module changed either of those behaviors. If I'm reading you right, you were getting Cache-Control:no-cache, must-revalidate with an anonymous user? I would check to make sure you actually had caching enabled in your test ;)

I also made another test after this one to see if i18n caching still functioned. As expected, language was cached just fine. Of course I would expect this anyway since it's only doing a redirect and not trying anything fancy like trying to detect when to load cache, etc. It's really no different than if someone typed in the URL and hit enter. Simplicity is sometimes the best solution ;)

I think I missed a step in my description above - you need to clear the page cache (or let it expire) after changing the code. Before that you might still be served pages from the cache (although you won't get the $_SESSION variable set either).

Here's what I saw exactly, starting from a fresh installation of Drupal core with the page cache turned on:

1. Reload the homepage a few times. The headers will be:
   

   Cache-Control: public, max-age=86400
   

   (max-age could be different depending on the expiration settings you're using)

   You might get a 304 response with the above, or a 200 response with X-Drupal-Cache: HIT also (this depends on what method you use to reload the page). Either way it means you're getting pages from the cache.

2. Apply code changes to set a $_SESSION variable in hook_init().
3. If you reload the page a few times now, you'll still get the cached results from above. But you'll also see that the code to set $_SESSION was never run yet.
4. Clear the page cache.
5. Reload the homepage again and make sure your reload hits the server. Now you'll see this:

   Cache-Control: public, max-age=86400
   X-Drupal-Cache: MISS
   

   This is the request that sets the $_SESSION variable and gives you a session cookie.

6. Reload some more. All subsequent requests have:

   Cache-Control: no-cache, must-revalidate
   

   and therefore are not cached.

Arguably it's a bug that step #5 sets the Cache-Control header to "public" (seems to be a side effect of $_SESSION not being populated at the beginning of the request even though it was populated by the end). It didn't make a difference for my testing, but if you have some kind of external cache in place that ignores session cookies (?), I can kind of imagine it continuing to serve pages from that cache for however long max-age is, i.e. one more day in this example.

But the key point is that the next page request which reaches Drupal after that will start setting Cache-Control: no-cache, must-revalidate and won't allow that user to see cached pages anymore as long as they still have something in $_SESSION.

I'm not sure if this is all documented anywhere directly, but from the code and comments at drupal_session_initialize() and drupal_session_commit() you can see that it's clearly intending to treat anonymous users with a session in a similar way as it treats authenticated users, which definitely makes sense to me.

Hmm I think I see this bug now. I was able to reproduce what you were saying and get it to never show a cached page again. I'm now trying the same theory using Cookies instead and with hook_boot rather than hook_init. So far, this seems to persist the way I thought the session var would, but is giving cached pages correctly. Whats your take on using Cookie or LocalStorage instead of Session? I changed to using hook_boot because with hook_init, the cookie was not being set (the same way session var wasnt being shown in the tests above). Changing to hook_boot seems to work around that quirk.

The modification with Cookie seems to work well. If you can find a way to break it, that would be useful (besides disabling cookies in the browser of course ;) ). I have modified the post above with the new snippet that uses hook_boot.

A cookie plus redirect in hook_boot() sounds to me like it should work, since hook_boot() runs even on cached pages.

If an external cache (like Varnish) is being used it's a little more complicated since what you want there is for the initial request to always bypass the cache (so it can reach Drupal) but other requests to use the cache whenever possible. It might depend on Varnish configuration, but you'd probably at least want to replace setcookie('drupal_locale_browser_detected', rand(1,10000)) with something like setcookie('drupal_locale_browser_detected', 1) -- if each user has a different cookie value, that tends to mean Varnish won't let them share a cache with each other, and there's probably no good reason to deliberately prevent that?

But if you're just using the built-in Drupal page cache, it seems good.

I actually meant to set the cookie to TRUE (the original setup had that anyway). I used Rand just to debug if it was using the same cookie value or a new one when open/closing tabs and browsers, etc. It should be TRUE (AKA 1) or perhaps the Language code like cz-hans, etc.

David Rothstein is right:

https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/_dru...

See:

  if (!isset($_COOKIE[session_name()]) && $cache_enabled) {

That clearly does not allow serving cached pages when a SESSION is present and putting something in $_SESSION removes the cache indeed.

So yes usually a cookie works best and then ensure that pages _always_ redirect when there is e.g. no language prefix and that page is never cached.

Hi all,

Maybe I did something wrong but a custom module with the code from #31 didn't produce any result in my case, but #27 did solve the issue.

FYI: Patch in #20 works great. The Global Redirect module provides a good workaround for the caching issue:
Language negotiation detects browser language (non-cacheable page), Global Redirect redirects to the page with the path prefix (cacheable page), and it does this in hook_init, so before a lot of the hard work is done.

[to be deleted]

#27 did the trick for me

The patch at #20 did not help, the module #27 worked. Thanks.
Edit: I uninstalled the module, cleared caches, turned off "Cache pages for anonymous users" on the admin/config/development/performance page and the language detection works. Maybe the patch works after all...

"Quick Fix" help me, thanks!
Drupal 7.59

In general, you may need to disable custom language provider also:
unset($negotiation_info[YOUR_CUSTOM_NEGOTIATION_PROVIDER]['cache']);

I'm having a similar problem, that it's driving me mad. Sometimes, and only for a certain period of time, and only for anonymous users, the content of a page generated by a view is showed in the wrong language, instead of Spanish, that is the default language, the content is in English, the secondary language. And when I click on another link on the site the language is right.

I have tried all the different methods and patches talked in this thread, also disabled cache for anonymous users, and even installed the module Cache Exclude to ignore caching that url.

It's also quite difficult to debug this issue, because it only happens from time to time, and for a limited time. This is the url where I'm having problems.

Can anyone help please? Anything else that I could try?