Endless loop with i18n

I've never used "I18n". I've no idea even what it does (on a code level)... I understand its some kind of translation based thing - but I often struggle with English as my first language ;-)

Unfortunately, I'm on holiday for 2 weeks from tomorrow so wont have a chance to even look at it for about 3 weeks... Sorry!

I marked the 3 lines and it's ok now. I'm note sure the purpose of this function, although I disabled it, but it still works fine. I open http://localhost/drupal5/node/3 and it redirects to http://localhost/drupal5/en/my_test_path

    // If current path is also the frontpage, redirect to http://www.mysite.com.
    //if (!empty($_REQUEST['q']) && drupal_is_front_page()) {
    //  drupal_goto('', $query_string, NULL, 301);
    //}

This is odd - this function basically tests if we're visiting any page within the website and then checks if it is the frontpage (eg, you might set example.com/node/12 to the frontpage - or even the default example.com/node to the front). This check simply tests if you're visiting a "frontpage" url which isn't the frontpage itself and redirects.

What URL Structure does I18n use?

Confirming this bug.

i18n URL structure is (when "cs" is my default language)

/ - frontpage in cs
/cs - frontpage in cs (this is duplicate with /)
/en - frontpage in en
/lang - frontpage in any language

Ohhhh so with the language functonality it simply checks the last argument to modify the language otherwise it uses the default, however '/', '/en' and '/[lang]' are all marked as frontpage so they will redirect... Hmmm...

Maybe we need to implement a language check (ie, if i18n is enabled, do a different algorithm, somehow ignoring the last argument if its a language arg)...

Nope. The url is:

/node/123
/cs/node/123 (duplicate with ^ when cs is default language. the above should be default - /cs/node123 should redirect to it - or this should be configurable?)
/en/node/123
/lang/node/123

ok - so its the first argument... :-)

Looks like i18n plays with $_GET during its hook_init...

i would also like to use Global Redirect with i18n.
I am willing to test.

This is a patch submitted my Jax in issue #182306 which looks like it fixes the i18n incompatibility... Can people confirm this solves the issue?

I've attached Jax's patch here for convenience...

subscribing

I tested the patch and the following is the result:
1. If the user is not login, then the default and non-default language won't get endless loop
2. If the user is login, default page is ok, but non-default language will cause a endless loop (only frontpage)
3. The URL of the language switcher (from i18n) is not changed. i.e. If current language is en, then the url in the language switcher is http://localhost/other-language/node/xxx

btw, how about to put a new development snapshot for users to test ? I think not everybody has the environment to use cvs, patch commands.

Good idea about the dev snapshot. I'll look into doing this at some point in the near future. My life is a little busy right now ;-)

Ok, the problem is still there. The issue is when you send q=nl for the dutch front page for example. You can fix it by adding an if here:

   // If current path is also the frontpage, redirect to http://www.mysite.com.
    if (!empty($_REQUEST['q']) && drupal_is_front_page()) {
      if(i18n_get_normal_path($_REQUEST)) {
        drupal_goto('', $query_string, NULL, 301);
      }
    }

But that isn't the most elegant solution. I'll try to provide a nicer patch this week.

is this what you meant?

  // If current path is also the frontpage, redirect to <a href="http://www.mysite.com." title="http://www.mysite.com." rel="nofollow">http://www.mysite.com.</a>
  if (!empty($_REQUEST['q']) && drupal_is_front_page()) {
    if(i18n_get_normal_path($_REQUEST)) {
      drupal_goto('', $query_string, NULL, 301);
    }
  }

This makes no sense to me because the i18n_get_normal_path simply checks the path and returns the source path (taking into account prefixes)... In fact... that doesn't look right at all as that function expects a string and you're passing an array.

I should be $_REQUEST['q'] not $_REQUEST

Ok well that makes more sense syntactically however I'm still not sure of the logic because surely just wrapping an "if" around that statement will always return true as the function always returns the path... These are the two relevant functions...

function i18n_get_lang_prefix(&$path, $trim = FALSE) {
  $exploded_path = explode('/', $path);
  $maybelang = array_shift($exploded_path);
  $languages = i18n_languages();
  if (array_key_exists($maybelang, $languages)){
    if ($trim) {
      $path = trim(substr($path, strlen($maybelang)),'/');
    }
    return $maybelang;
  }
}


function i18n_get_normal_path($path) {
  $prefix = i18n_get_lang_prefix($path, TRUE);
  if(!$prefix || _i18n_is_bootstrap()){
    // If bootstrap, drupal_lookup_path is not defined
    return $path; 
  } // First, check alias with lang
  elseif($alias = drupal_lookup_path('source', $prefix.'/'.$path)){
    i18n_get_lang_prefix($alias, TRUE); // In case alias has language
    return $alias;
  } // Check alias without lang
  elseif($alias = drupal_lookup_path('source', $path)){
    i18n_get_lang_prefix($alias, TRUE);
    return $alias;
  } 
  else {
    return $path;
  }
}

This is getting rather confusing and I have a lot on right now (looming deadline) however in about a week or two I'll have time to investigate this properly...

Until then can someone confirm if Jax's theory works?

Also - could that second 'if' statement be merged into the first one to make it a 3-parter?

subscribing

There are more issues with my code. For example, with http://example.com/nl is_front() returns true, but you don't want to redirect in that case, the content is different (it's in another language). I'lll try to come up with a reworked version during the weekend of next week.

Hi
I also have this bug, any updates since 15 nov? I think by solving this issue it will be easier to make global redirect work with D6. In D6 parts of i18n i built in to the core.

BTW: i18n can also use: http://lang.example.com not only http://example.com/lang and it can use your browser lang. to detect front page/article language to use on users not logged in. We just have to make sure we have tested all options, and YES I will help you out with testing the patch :)

--
Stein Magne

Hi
Has anyone tested this bug i D6? If not I will be more then willing to check it out.

--
Stein Magne

I haven't had any time to test it as of yet - so if you can, that would be great. I assume it is still an issue in D6...

As stated above the patch doesn't work as it should. Also, it's a D5 patch, there are a lot of i18n changes in D6 so it won't be compatible. When I have time to spare I'll try to look into this.

I have been looking more into i18n on D6 the last days. And as some parts of i18n made it into D6 you don't really need to install i18n to test this bug, all you need is the global redirect module installed.
- Global redirect module 6.x-1.x-dev ver. 30.12.2007
- Drupal 6 dev snapshot 26.1.08

Same problem. Should I create one separate issue on 6.x version?

--
Stein Magne
http://smbjorklund.com

Well... PERSONALLY I believe that as i18n is part of D6 core then it should be fixed in D6 and backported to D5 (as i18n is optional in D5)...

I am aware of the issue and plan on looking into it however I have absolutely no spare time on my hands right now...

So is it your opinion that this is an bug in the i18n-part and not global redirect?

I wouldn't say there is a bug in either module really... Its just GR needs to be made aware that i18n handles URL's differently to standard Drupal paths.

Yes, please open a new issue against Drupal 6 itself or Global redirect, since Drupal 6's code is different to i18n for Drupal 5. Please post your issue URL here.

This issue is now posted againt Drupal 6 http://drupal.org/node/216271

I just came up with this code that works for me (i'm using Drupal 5.7):

<?php

    // If current path is also the frontpage, redirect ...
    if(function_exists('i18n_get_normal_path')) {
	  if (str_replace(array('/','node'), '', $_REQUEST['q']) == i18n_default_language()) {
	    drupal_goto('', $query_string, NULL, 301);
	  }
    }
    elseif (!empty($_REQUEST['q']) && drupal_is_front_page()) {
      drupal_goto('', $query_string, NULL, 301);
    }

?>

sorry, no patch :(

Here's a patch for roborn's code.

This patch worked on my site. Although with some glitches.

I have english and spanish enabled, being spanish the default language. /node is the frontpage.

/en, /en/node, /es, /es/node, and / don't get redirected. Only when I'm in english / redirects me to /en.

Here is another patch, it appears to work fine on our site. (Drupal 5)

Please let me know if it works

Best,
Jacob

Jacob - thanks for the patch, could you please try to run the diff using the -up argument. I am finding that one a little hard to understand.

It looks VERY promising though!

Here it is again.

Sorry if the format was strange...

subscribe (sorry but I want to keep track)

#33 seems to work for me, but it's not 100% tested: I only have one language at the moment.

I am using a fully updated 5.5 and both i18n (en,gr,it) and globalredirect and I had some of the problems that are described above. The only real problem though was that for IE it was redirecting automatically to the language (although not selected in the multilingual settings) and that was returning a 404. No problems with Safari or Firefox.

I tried the above patches and still not working. I removed the globalredirect and it works like a charm.

Oh, the site is http://aerides.eu

i18n_get_normal_path() aso. are i18n-specific functions. However, there are also Localizer and #translatable modules. Please do not solve this bug for i18n alone. My thoughts on this:

While I think that part of the problem is GlobalRedirect's use of $_REQUEST['q'] instead of $_GET['q'], a possible solution might be to extract and test the first path element against the returned list of http://api.drupal.org/api/function/locale_supported_languages/5 If isset($locales['name'][$path_element]) returns true, it could be stripped in front of running GlobalRedirect's tests.

However, that might be en expensive operation. i18n & Co. already strips the language prefix from $_GET['q'], assigns the locale, and overrides the front page variable accordingly. But all of this happens in hook_menu(), so the next best question is, if GlobalRedirect really needs to run in hook_init().

Going further along this road, both i18n & Co. and GlobalRedirect would run in hook_menu() - thus, GlobalRedirect would need to have a higher module weight than i18n/Localizer/translatable, so language negotiation happens before GlobalRedirect is invoked.

sun - I agree with what you're saying, however GlobalRedirect's intention is to be lightweight... I' not sure I feel its fair on the majority to move the functionality to a later stage in bootstrap so the minority can get redirection properly...

How heavy would it be fore GR to detect if there is more than 1 language and to figure the stuff out for itself if that were the case?

But I really do agree that a solution should apply to as many language packs as possible.

Oh yeah, Sun, the reason we use $_REQUEST rather than $_GET is for (at least) the reason that Drupal plays with $_GET so it doesn't truly represent the URL... For example, go the frontpage of your site (eg, http://www.example.com/). Now if you were to have this code in a hook_init somewhere...

print_r(array('req' => $_REQUEST['q'], 'get' => $_GET['q']));

You'd see this at the top of the page...

Array ( [req] => [get] => node ) 

$_GET['q] has been modified by drupal to represent the internal source path - it no longer represents the URL.

Ok I've produced a patch similar to Jacobs in #35...

Concerns... Imagine this:
1) Enable English and French (en and fr)
2) Create a node with an alias - eg - "my-nice-page" and mark it English
3) Go to it, you should get redirected to 'en/my-nice-page'.
4) If you were to visit it with 'fr/my-nice-page' then from that point on, fr is your default language and yet the english content is being shown under a different URL.

Any thoughts?

I agree with what you're saying, however GlobalRedirect's intention is to be lightweight... I' not sure I feel its fair on the majority to move the functionality to a later stage in bootstrap so the minority can get redirection properly...

Yes. You wouldn't be able to maintain this. And that's also the reason why GlobalRedirect should not implement this directly. Let's turn it around and let other modules indicate or maybe even alter the path before GR tests it. Would that work out?

Hmm..... another thought:
Even if the language negotiation happens later on, this doesn't mean that we can't strip the language prefix and try to find an alias. Although this means that GR won't work for the front page in other languages, it will work for the default language and all other paths with all of the multilingual solutions currently available. IMHO, that would be a reasonable compromise. Attached patch tries to implement this. The same approach would work-out for D6, see language_initialize for a similar code snippet in D6.

Note: Needs testing. I wasn't able to test this patch on a live site.

Re-rolled patch after creating the same one for D6.

Subscribing.

Subscribing

45 folllow-ups and no one is able to test this crucial patch? Please do not only subscribe, please test, and report back whether this patch works for you, so this bug can be fixed. Thanks.

You are right sun, and here's my result when applying your last patch (#44):

/sites/all/modules/globalredirect$ patch < globalredirect-DRUPAL-5.patch
patching file globalredirect.module
Hunk #1 FAILED at 18.
1 out of 1 hunk FAILED -- saving rejects to file globalredirect.module.rej

I am by all means a total beginer on patches (this is actualy the second time I ever patched something), i ran it from the globalredirect directory like you can see, it seems to find the file so i guess that part is ok. However I dont know what the rest means much,

I attach both files it created in case they are useful (i had to zip them because their extensions arent alowed).

@manuee: All patches are against the latest development snapshot (actually, current DRUPAL-5 branch in CVS) and cannot be applied to the latest official release.

My appologies again... like I said, I am a complete beginner on these issues...

OK, so I downloaded globalredirect-5.x-1.x-dev.tar.gz (I hope I am using the right one this time), applied the patch (no errors returned), and enabled the module with i18n running.

It seems like now the front page is displayed properly, you can change languages fine. However, it seems it's the only page working now, if I browse around to any other page, I get the same error that we were getting only on the front-page before the patch.

note: I dont have clean urls enabled, but i think it's not relevant to this issue.

I cannot disable the module with the patch applied so I'll go now and fix that in the database.

I dont have clean urls enabled, but i think it's not relevant to this issue.

Thanks, I didn't test that. This sounds like a possible cause for me.

Thanks, I didn't test that. This sounds like a possible cause for me.

I went ahead and tried enabling clean urls, applying the patch, and enabling globaredirect - I got the same result as described in my last comment.

I dont believe Clean URL's will cause any problems as GR looks at the 'q' part of the query. Without clean URL's, $_REQUEST['q'] is simpy there however when you use Clean URLs then Apache rewrites the bit of the URL after the first slash into the 'q' argument for you.

Hi all
System:
- Drupal 5.7
- PHP 5.1.x
- Apache 2.0.x
- Clean URL on
- Language English and Norwegian (nb)
- Default language en
- Frontpage http://example.com/node

Rolled patch #44 on my test server, and I still get endless loops on every page with http:example.com/nb

Any thing I can help you debug?

Cheers
Stein Magne

This also affects Drupal for Facebook. http://drupal.org/project/fb

I'm not in favor of the patch in #44 because it is i18n specific. The solution should address any sites that use custom_url_rewrite().

You can tell that custom_url_rewrite is in effect if $_REQUEST['q'] is not equal to drupal_get_normal_path($_REQUEST['q']).

I have not tested this in every scenario, but something like this might do the trick.

Hi
+1 that this fix must be more general. It is prob. affecting more modules then i18n, fb and prob localize (not checked).

The patch does have a wrapped line that start with "h)" or you have typo in your code:
patching file globalredirect.module
patch: **** malformed patch at line 41: h)

Cheers
Stein Magne

Sorry, had to copy that patch from an ssh session. Linebreaks were not preserved. Hopefully this one is better.

Patch looks interesting - can anyone else test with systems effected by this problem?

What about a simple use of the $i18n_langpath global ??? I just took a short lazy look, thats why im not sure if my approach is silly but its seems to work in D5 with i18n and global redirect ...

<?php
  global $i18n_langpath;
  // If current path is also the frontpage, redirect to example.com.
  if (!empty($_REQUEST['q']) && $_REQUEST['q'] != $i18n_langpath && drupal_is_front_page()) {
    drupal_goto('', $query_string, NULL, 301);
  }
?>

$i18n_langpath stores the language prefix, e.g. 'de', 'ru', 'en', 'it'

The snippet above in hook_init context:

<?php
/**
 * Implementation of hook_init().
 */
function globalredirect_init() {
  if(isset($_REQUEST['q']) && function_exists('drupal_get_path_alias') && !isset($_REQUEST['destination'])) {
    
    // Get the Query String (minus the 'q'). If none set, set to NULL
    $query_string = drupal_query_string_encode($_GET, array('q'));
    if (empty($query_string)) {
      $query_string = NULL;
    }

    global $i18n_langpath;
    // If current path is also the frontpage, redirect to example.com.
    if (!empty($_REQUEST['q']) && $_REQUEST['q'] != $i18n_langpath && drupal_is_front_page()) {
      drupal_goto('', $query_string, NULL, 301);
    }

    // Trim any trailing slash off the end (eg, 'node/1/' to 'node/1')
    $request = trim($_REQUEST['q'], '/');
    
    // Check the path (eg, node/123) for an alias. If one is found, redirect.
    if ($alias = drupal_get_path_alias($request)) {
      // If alias is different to the request, redirect...
      if ($alias != $request) {
        drupal_goto($alias, $query_string, NULL, 301);
      }

      // If the request produced by the trim above differs to the request then redirect (basically, de-slash)
      if ($request != $_REQUEST['q']) {
        drupal_goto($request, $query_string, NULL, 301);
      }
    }
  }
}
?>

derjochenmeyer: your patch is specific to the i18n module. Dave Cohen's is not, and therefore is better.

subscribe

Lol - about 10% of the posts on this thread have been 'subscribe' ;-)

Dave's patch appears to make sense, however I'm personally snowed under right now to test it... I'd appreciate it if this could be tested.

I've tested Dave's patch and it seems to work well. But it has the same small problem as my approach. Both

www.mysite.com
www.mysite.com/defaultlanguage (www.mysite.com/en if en is default point to the same page)

This excludes the benefit of globalredirect from the default language frontpage.

Example (Dave's patch running):
http://www.iwanttospeak.net
http://www.iwanttospeak.net/en

Oh, interesting that you cite that as a problem. I can see how in your case it's not desireable. But in the case of Drupal for Facebook, I need both of those URLs to work. So if we "fix" that, let's please make it configurable, or put the fix in the i18n code.

Here's how DFF uses custom_url_rewrite... Let's say our site is hosted on www.mysite.com, so a request for www.mysite.com/node/123 does what you'd expect. The site also hosts Facebook canvas pages. This means visiting apps.facebook.com/mysite/node/123 will result in a call to www.mysite.com/fb_cb/1/node/123 (that is, a request for node/123 served up by facebook application 1). Potentially, the site can host more than one facebook app, so www.mysite.com/fb_cb/2/node/123 could be a valid request.

In my case, all three URLs (node/123, fb_cb/1/node/123 and fb_cb/2/node/123) have different meaning and Global Redirect should respect that they are different.

In your case, consider three comparable URLs (node/123, en/node/123 and fr/node/123). It happens that node/123 and en/node/123 produce the same output. But that's really the logic of i18n, I don't think global redirect should have to be aware of it.

I actually agree with Dave on that point that i18n is actually CAUSING the dupe pages by not redirecting the default language pages itself. Maybe this is an issue which should be raised with i18n?!

While i18n uses custom_url_rewrite(), neither #translatable, nor Localizer use it.
However, you might be right in that we need to additionally fork drupal_init_path().

Jup its probably not an issue of globalredirect alone ... but as i understood, it could result in bad search engine ratings, right?

Isn't the main goal of gloabalredirect to eliminate duplicate pages from a SE point of view? Of course that collides with your usecase, maybe with others.

I think that this could be fixed with a module_exists('i18n') conditional statement ... it would harm nobody ... will investigate ;-)

I know this might add a little "weight" to the module... but what do people think about this? In the function, towards the end, we create a "module_invoke_all" and have a "hook_globalredirect". We can then ship other modules along with GR which can be enabled per-situation... Eg, create a bolt on for i18n, and another for facebook, etc...

Personally I'd prefer a "one hammer for all nails" approach if it can be elegant and efficient though...

subscribe

Any progress on this issue?, it is still critical...

I don't think we should make changes that brake other applications (like FB) to make this work, but on the other hand we also have this problem in D6 so if we take that into consideration it sort of make sense to fix it against i18n since this is in the D6 core..

Cheers
Stein Magne

Not that I'm shrugging off the issue here - but I have absolutely NO experience with i18n. I really am only able to help by testing patches, I dont have the experience with i18n to actively solve this.

I also do not have significant experience with i18n. However, I do have a client using Drupal for Facebook and I've applied my earlier patch to their site. I'd like to see this get checked in. If there's anything more I can do, let me know.

This patch feels critical to me, but I can understand the argument that it is not. It only happens on sites with custom_url_rewrite, which is not most.

subscribe

roborn, thank you for entry #30 - that fixed the issue for me in Drupal 5.7.

Specifically, when a visitor went to http://www.example.com/en, then the front page would display as normal. Going to http://www.example.com/de caused the endless redirect.

#59 im changing the status to "reviewed" to get some attention. I tested Daves patch and it works without problems. My previous concerns #65, that the frontpage has 2 urls pointing to it are minor i think. A search engine should be smart enough to detect obvious language specific paths...

which patch should we test for 5.7 together with which version of global redirect - i lost a little bit the orientation.

or went something already into dev? if not: maybe as a pragmatic way first only a i18n-specific solution into dev + later the general patch?

greetings momper

Drupal 5.9 - same problem, i.e. www.mysite.com is fine, www.mysite.com/en is ok too (en is the default language) but www.mysite.com/es goes into infinite redirect loop. Other pages e.g. www.mysite.com/es/something seem to work fine.

Would someone please confirm what patch or change is required please? Reading through this thread it seems there have been several patches, and anyway don't know if tried with Drupal 5.9. Thanks.

Thinking more... how about a generic fix that detects infinite redirect? If it happens for any reason it's pretty nasty (whether this problem, some other module's fault, etc) so would this be feasible? ...

* Remember (cache) that we've issued a redirect (by user session? IP address? how?)
* Check if we're going to redirect to somewhere we just redirected this user to and bail out if it looks like a loop.

Arguably this should be done by drupal_goto() ?

I've implemented derjochenmeyer's suggested change from #61 and that seems to work (Drupal 5.9)

The problem is more complex :( Language codes in the url break the basic functionality of this module, e.g.

http://www.axlan.com/node/102 - gets redirected correctly but...
http://www.axlan.com/en/node/102 does not get redirected (it should.)

(those are live links)

It looks like the method of checking for an alias doesn't work when a language code is given in ther url.

EDIT: The above links now work as required because I'm running my heavily modded version of globalredirect.module see #86/#89

Setting proper status.

Sorry, I will probably not be able to get back to this in the next week. However, your redirection-loop proposal sounds sane and could be added regardless of a proper fix for this bug (using $_SESSION of course). I still think that we can achieve a solution similar to #44/#59 that will work with any kind of custom_url_rewrite(), like Dave suggested.

Ok, let me know if you need more infor re. the langauge issues - I appreciate that if you've not got a multi-language site set up its difficult to test.

Do you want me to log #82 as a separate issue? It may be related to the infinite redirect problem, and possibly a single fix will sort out both problems. To summarise:

node/100 has alias of my/friendly/url
site.com/node/100 should redirect to my/friendly/url (currently works)
site.com/en/node/100 should also redirect to my/friendly/url, or more correctly to en/my/friendly/url

Currently the second case above is ignored by Global Redirect.

Looking at i81n module there is this function:

/**
 * Rewrites path with current language and removes prefix if searching for source path
 */
function i18n_url_rewrite($type, $path, $original){
  if ($type == 'alias' && !strpos($path, '://') && !i18n_get_lang_prefix($path)){
    return $path ? i18n_get_lang() . '/'. $path : i18n_get_lang();
  } elseif ($type == 'source') {
    if ($path == $original) {
      return i18n_get_normal_path($path);
    } else { // Path may have been dealiased but still have language prefix
      i18n_get_lang_prefix($path, TRUE);
      return $path;
    }
  } else {
    return $path;
  }
} 

So I think you need to call i18n_url_rewrite() to remove the language prefix from the path before looking for an alias to that path. Probably should first check for an aliase which includes the language prefix as that might also exist. I'll try this quickly myself and let you know.

Re. the loop detection possibility...

I just took a quick look at the source for drupal_goto() and definitely think it would make more sense for any detection of infinite-redirect to be done in there so that it would work in all situations. I might try creating a patch... thinking about it, its not so trivial. A simple redirect loop might be easy to detect, but what about A->B->C->A etc? Also, I'm not sure where to reset the "redirect" status, i.e.

in drupal_goto() ...
get old redirected url(s) from user session data
if (new url matches old url and time interval is small)
bail out
else
store new url and timestamp in user session data
redirect to new url

somewhere else ...
clear old redirected url(s) from user session

Where should the "somewhere else" part best be done? Could be done in drupal_goto as well, but then some might never get deleted? Or, just rely on Drupal internal cache clearing to discard the old data? In summary, not such a trivial excercise as I first thought, maybe better to just fix the underlying problem with Global Redirect vs i18n.

After some experimentation, it does apepar that i18n_url_rewrite() is what's needed. But, this has set me thinking, the whole point of your module is to prevent two or more different urls being seen (by Google etc.) as pointing to the identical content. This raises yet more issues with multi-language sites, because mysite.com/somewhere and mysite.com/en/somewhere will point to the same content (when 'en' is the default site language). Furthermore, depending on i18n language negotiation settings, mysite.com/en/node/100 and mysite.com/es/node/100 can both point to the same node etc. It might be the identical content, or might deliver distinct content per language (menus etc would probably change at least.) So for Global Redirect to work as required, it would need to:

(a) add the default language code to all urls which don't have one OR
(b) remove the language code from urls if its the site default language.

Option (a) seems best, that makes sure that what Google sees with be consistent, and would allow the site default language to be changed in the future - Google's set of urls would remain valid.

To further complicate matters, aliases may or may not include a language code. So, the logic needs to be more complex depending on whether the original path has a language code or not, whether the alias has a language code or not, and so on. I still haven't got my head round the full set of possibilities. Maybe it would be easier to ensure that all aliases include a language code.

I've ended up doing what amounts to a partial re-write of your Global Redirect module. This fixes the "infinite redirect" problem, but also addresses the issue of duplicate urls for the same content being seen by Google etc. i.e. site.com/page and site.com/en/page where 'en' is the default language. Basically all urls without a language code (original requests and aliases) will end up redirected to the corresponding url with a language code included. The choice of which language code gets added is made by i18n in the usual way.

The partial re-write was needed because the logic needed to change to consider a multi-language environment. This version *should* work fine when i18n is not installed, but I haven't tested it in that environment. I have tested it with a dual langauge site (en/es) and it seems to work as it should.

If you would prefer not to use / support this version I will understand! In that case just let me know, and I'll create a separate project - I already have a couple of other projects for multi-language sites.

Anyway, here it is attached. It's in live use at www.axlan.com.

Please do not attach compressed archives. Archives are the source for all kind of problems. Even if it is a rewrite, just post a patch here.

btw: Creating new, duplicate projects just because a bug or feature XYZ is not fixed or added in an existing project ZYX (yet) - is strongly discouraged. Please take a moment and think about users of Drupal that want one feature but are then faced with evaluating and deciding among duplicate modules.

Thanks

Noted. I tried to attach the uncompressed version but drupal.org wouldn't let me. I'll do a patch file in a moment.

Here's the diff file.

By the way, I didn't spend time figuring out exactly why the "infinite redirect" problem occurs, because I needed to add extra logic anyway to handle the issue of duplicate urls for default language, and I figured the problem would go away once I'd done that (which it did.) But, I think the problem is this:

1. If for example i18n thinks that the user's chosen language is 'es' (with site default 'en') it redirects site.com requests to site.com/es
2. The page site.com/es passes as "true" for the drupal_is_front_page() test so Global Redirect sends you back to site.com (1) - infinite loop.

That would confirm that earlier patches (#61) above should work fine to fix the "infinite redirect" problem, but won't handle the problems I described in #82 and #85.

Also, regarding #42, i.e. a url of mysite.com/fr/node/123 where node 123 is actually not in French.... There is a basic anomaly: the path starting /fr means the visitor wants to see stuff in French wherever possible. The part /node/123 means the same person is specifically asking to see node 123, regardless of what language node 123 is written in. There's no solution! You could look for a French translation of node 123 and redirect to it if it exists, but that would be a different node, not what they asked for and maybe not what they want to see. Or, you could switch the path to whatever language node 123 is written in, but then other things (blocks, menus, etc) would switch language, again quite probably not what the user wants. I think handling this anomaly would be outside the scope of this module.

EDIT: (1) above confirmed. If you previously accessed a /es path then a subsequent request to site.com results in "HTTP/1.1 302 Found" and "Location: http://site.com/es". Interesting that it uses 302 rather than 301 - which is "right" in this context?

Netgenius, the patch in #59 has worked for everyone that's tried it. And it introduces no i18n-specific code. Please give it a try and report any problems before switching the direction of this thread.

I'm setting the status to be code needs review. The code in question being the patch in #59.

Patch from #59 worked like a charm for me.
Still the issue that has been mentioned here (but should be another issue thread, really):
node/34 gets redirected, whereas de/node/34 or en/node/34 doesn't.

Ok, agreed I'm off topic and will cease :) The original "infinite loop" bug was already fixed. My points in #83 and #85 (also made by others elsewhere here) are really separate issues. I personally needed a solution for all three issues, hence my partial re-write, it's offered for anybody that wants it. Sorry if I've upset anybody.

I have committed patch #59 to the DRUPAL-5 (dev) release and done some BASIC testing on a dev site with a single node.

This fixed the "endless loop" issue but does NOT solve the issue of i18n allowing a user to view an english node in german (and displaying the same content). I believe this is something which i18n should deal with as its very context sensitive.

Can you guys checkout the dev branch to see if it works?

For those interested in the changes - attached is a diff between DRUPAL-5 (the one I'd like you to test) and DRUPAL-5--1-3-1 (the most recently official release). This diff also contains a few fixes from other issues too (eg, the base path problem and the missing menu access checking function problem)

Thank you Nicholas.

Ok, I just ran into this problem with D6. Was trying to jump between an /eng & /fra versions and got an infinite loop again.

Disabled the module and there's no problem with internationalization.

Update, this was a dev version of the code that was older than I thought. Please disregard this.

Hi,

I install i18n version 6.x-1.9 and has this problem. I upgrade to 1.10 then it continue to have this problem.

The problem only happened when access http:/mywebsite/es/ , when I put http://mywebsite/es (without slash in the end) it works well.

Anyone has solution for this?

thanks.

hosais

The attached patch file fixes two problems. The first is the front-page redirect loop for i18n (i.e. when you go to www.example.com/fr/ and it redirects to www.example.com/fr/ endlessly), the second is the redirect loop that occurs by duplicating the language on non-frontpage content, e.g. www.example.com/fr/content becomes www.example.com/fr/fr/content and so on, increasing the number of language elements until the browser complains.

I should say that the patch fixes the symptoms. I think the problems could be better addressed.

Just changing status

Replacement patch, in correct format this time.

Changing status for the bot.

.

I am seeing this behaviour with Globalredirect and i18n in Drupal 6 as well.

I'm not sure if this is actually the same bug fixed in Drupal 5 or has more to do with the issue('s?) covered in:

• #153950: Endless loop with i18n
• #832862: hook_url_inbound_alter breaks the module
• #1445934: Fix logic of module, broken when other modules implement prefixes.

This does seem to be a problem in Drupal 7 as well.

Steps to reproduce:
http://drupal.org/node/1580716#comment-6003412

It seems like work around this by setting the language detection method to "URL" (admin/config/regional/language/configure), before enabling global redirect.

I got the same problem with the newest version of global redirect.

How to reproduce:
One Domain per Language (LANGUAGE_NEGOTIATION_DOMAIN)
Create a node with no language and set a aliases for all languages.
Visit this node -> endless redirect loop

Problem:
In case of language domain negotiation it checks if there is a alias for another language for this node. Because the alias is for all languages, this is always true and it redirects you to the other domain. Same happens again for the other domain.

Simple Patch attached.

Similar patch is needed for 7.x-1.x

@kfritsche did you test your patch against - https://drupal.org/node/1540064 ??

EDIT: Ohh it's drupal 6.x, sorry didn't notice it when I have been looking for solution.

Patch should be rolled against 6.x HEAD. @pwiniacki if you read #107 careful will you see he also indicate this is problem in D7.

Guys, is a fix for 7 in the works at all?