Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I just peeked over this module, since i am currently developing something similar. Now that i solved it, i want to share some.
As i can see for 6.x, this module utilizes its own redirect header statement. I suggest to use drupal_goto('path', drupal_get_destination()), since it will shutdown drupal's flow correctly. Also drupal_get_destination() includes other query-variables, not only the internal drupal-path, as well as messages that has been set before.
The mentioned recursive redirect loop can be avoided by checking the path against 'user' in arg(0).
Comments
Comment #1
Bevan CreditAttribution: Bevan commentedRedirect gets in infinite loop as anonymous user hitting node/add with i18n path prefixes.
Comment #2
Bevan CreditAttribution: Bevan commentedThis version of the patch employs
drupal_get_destination()
, but notdrupal_goto()
or the check forarg(0) == 'user'
. This enables the module to work with i18n language prefixes, and doesn't infinitely redirect.Without using
drupal_get_destination()
while i18n path prefixes are enabled, the module redirects to, for exampleen/en/node/add
. Note the double-occurrence of the language prefixen
. This results in a 404 Page not found error.Comment #3
deekayen CreditAttribution: deekayen commentedcommitted to DRUPAL-6--1 and HEAD
@Bevan, your patch didn't conform to http://drupal.org/patch/create, but I used it anyway since it was so small.
Comment #4
deekayen CreditAttribution: deekayen commentedFrom Bevan:
@Bevan: it's your patch... roll it back?
Comment #5
deekayen CreditAttribution: deekayen commented@Bevan: I added you to CVS so you can just fix it instead of having to roll patch. I ended up committing it to DRUPAL-5, DRUPAL-6--1, DRUPAL-7--1, and HEAD, so this makes quite a mess.
Comment #6
deekayen CreditAttribution: deekayen commentedNevermind, I rolled it back. Is there still some underlying problem from the original issue submission that needs to be resolved?
Comment #7
Bevan CreditAttribution: Bevan commented(#518534: Redirects to /r4032login instead of old path was marked duplicate.)
I haven't tested whether #518534: Redirects to /r4032login instead of old path is a result of this (#339120: Redirect issues) change or not. I'm merely wondering. I would expect that I would have noticed a bug as significant as this in developing the patch. Perhaps there are other or more obscure causes? Have you tested?
I don't have time to work on it this week, but will probalby get around to it sooner or later...
Comment #8
Bevan CreditAttribution: Bevan commentedIt turns out
drupal_get_destination()
is indeed the cuprit here. This needs rerolling. Possibly we should state on the project page that this module is not compatible with i18n path prefixes, in the meanwhile?Comment #9
ianchan CreditAttribution: ianchan commentedsubscribe
Comment #10
Bevan CreditAttribution: Bevan commentedI think the cleanest way to fix this is to check for language prefixes in
$_GET['q']
. But this would require quite a bit of fairly ugly code, and I couldn't find the code that does this already in Drupal to see if there is something re-usable.Comment #11
Breakerandi CreditAttribution: Breakerandi commentedi18n redirect still not working on 6.12, will it be fixed soon?
Comment #12
Bevan CreditAttribution: Bevan commentedBreakerandi; Patches welcome
Comment #13
Breakerandi CreditAttribution: Breakerandi commentedMy profession is to report bugs, I can't do anymore for the community because I know nothing about php.. sorry..
Comment #14
geraldito CreditAttribution: geraldito commentedI don' t get what's the point of #4 as for me patch from #2 fixed the redirection problem with i18n.
@#1: anonymous user hitting node/add with i18n path prefixes doesn't effect an infinite loop on my site (maybe because there is content creation allowed for anonymous users?)
@#1: anonymous user hitting /admin gets redirected to user/login?destination=admin so for me everything seems to be fixed with this patch.
Comment #15
mike-l CreditAttribution: mike-l commentedI find this bug too... and I see only 1 way how to fix this issue.
idea is:
remove laguage prefix.
in the
r4032login.module
$destination = drupal_urlencode($_REQUEST['q']);
i suggest to rewrite like this:
$destination = removeLanguagePrefixFromDestination(drupal_urlencode($_REQUEST['q']));
and add new function:
function removeLanguagePrefixFromDestination($destination)
{
$elements = explode('/', $destination);
$prefix = $elements[0];
$languages = language_list();
foreach ($languages as $lang)
if(strcasecmp($prefix, $lang->language)==0)
return substr($destination, strlen($prefix)+1); // remove prefix with slash
return $destination;
}
and everything works fine
any thoughts about this solution are welcome )
-----------------------------------------------------------------------------------
patch #2 doesn't work in my case
Comment #16
mike-l CreditAttribution: mike-l commentedi'm sorry but in my previous message patch.patch was not correctly generated .I recommited it in this message.
Comment #17
Bevan CreditAttribution: Bevan commentedmike-l, Please see the Drupal coding standards. Among other things
removeLanguagePrefixFromDestination()
should ber4032login_remove_language_prefix_from_destination()
.However I think there is a Drupal core API function for this – I think it's
arg()
with no parameters, ordrupal_get_normal_path()
.Comment #18
deekayen CreditAttribution: deekayen commentedI applied the general idea for a fix to DRUPAL-6--1, DRUPAL-7--1, and HEAD. Please try it so I can make a 6.x-1.3 release if it works.
Comment #19
vlad.k CreditAttribution: vlad.k commentedsubscribing...
Comment #20
John Franklin CreditAttribution: John Franklin commentedIs there a patch for this that I can apply to a 6.x-1.2 install? (I hate to run dev versions of a module on a production site, but backporting specific patches is usually OK.)
Comment #21
rolfmeijer CreditAttribution: rolfmeijer commentedIs the patch from #1087874: Multi-language site: When destination includes lang code, redirected URL get duplicated lang code useful?
Comment #22
lotyrin CreditAttribution: lotyrin commentedUpdating title. This is the concern of this issue, correct? If not, feel free to change it back.
I am also experiencing this problem.
Comment #23
lotyrin CreditAttribution: lotyrin commentedOkay, in addition to the query string getting lost, there is an assumption that a language's prefix is always the same as the language.
Writing a patch.
Comment #24
lotyrin CreditAttribution: lotyrin commentedHere are some improvements:
drupal_goto() should be used (to invoke exit hooks, save session state, etc.), and *can* be used as long as we unset the destination.
calling exit() denies modules that hook exit from having their hooks called. We don't need to call theme('page'), just return.
Wrote a new function that determines the proper destination, accounting for query strings and improving upon the language prefix fixing code (prefix testing was not accounting for the possibility of disabled languages, or of languages having custom prefixes). This code is inspired by code solving the same problem from logintoboggan, but differs in that it is smarter than logintoboggan.
A couple simple tweaks to logic flow and variable scope.
These fixes are against 6.x-1.x, but if we don't want them there, I can re-roll against a different head.
If these patches are accepted in one head, I will own re-rolling for other heads.
Comment #25
lotyrin CreditAttribution: lotyrin commentedForgot about
Comment #26
RobLoachHey lotyrin, would you like commit acccess to commit these patches? We would need to make sure they are persistent across both the 6.x and 7.x releases though.
Comment #27
BenK CreditAttribution: BenK commentedSubscribing
Comment #28
lotyrin CreditAttribution: lotyrin commented@Rob Loach re #26, sure. I'll make sure to get them in all heads.
Comment #29
marcvangendHi, what's the status here? The patches do not apply to 7.x-1.5 and the language fix that was committed in #18 does not work - at least not anymore. (Looks like it has something to do with leading/trailing slashes in the destination path.)
What's the best approach to get r4032login working on a multilingual D7 site?
Comment #30
deekayen CreditAttribution: deekayen commentedI just marked #1304864: Doesn't work in subdirectory install as duplicate of this because it's working on the same lines of code for a similar redirect breakage. I'm completely confused as to why there are 5 different patches working on totally different parts of the module for this issue and then why the issue was bumped to 7 when the proposed fixes are for 6, which is probably a good place for them to start. Can we focus on fixing the breakage first?
Comment #31
deekayen CreditAttribution: deekayen commentedIs perhaps the Redirect API a better way to handle this as a new dependency?
http://drupal.org/project/redirect
Comment #32
Herr Lehmann CreditAttribution: Herr Lehmann commentedThis a very straight forward approach, initially created in #1304864: Doesn't work in subdirectory install. I will have a look at http://drupal.org/project/redirect, to check how it can provide any help.
For right now I reworked my patch (for 7.x-1.x) and converted tabs to spaces. This is my first creation of a patch, so please be patient ;)
Comment #33
lotyrin CreditAttribution: lotyrin commentedI fixed these once in 6, I'll do so again in both branches.
Comment #34
lotyrin CreditAttribution: lotyrin commentedThe following have been fixed for both 6.x and 7.x (some were fixed in 7.x already):
Unicode characters in destination
Sites that are installed in subdirectories
Sites that don't have Clean URLs enabled
Sites which use language prefixes other than the default
Any futher problems should open new issues.
I'll let the other maintainers decide if these changes are ready for a stable release, but until then they're in -dev.
Comment #36
darthf1 CreditAttribution: darthf1 commentedStill having the multilanguage path prefix issue, with the latest dev (7.x-1.x-dev 2012-Apr-20)
nl/user/login?destination=nl/admin
results in redirecting me tonl/nl/admin
Comment #37
ddangel CreditAttribution: ddangel commented@ fcjversc
me too. But if you work with regex, you can fix this very fast.
[My urls are de/.../.../bla_word_with_de_bla, en/.../.../bla_word_with_de_bla, [lang_code]/.../.../bla_word_with_de_bla]
just change r4032login.module like:
and this should also works and looks better:
Comment #38
boran CreditAttribution: boran commentedWith a D/ multi-lang site with prefixes, I had to make the following changes to get it to work, first _r4032login_remove_language need fixing:
Then in r4032login_redirect() a leading slash had to be stripped:
Could someone else verify this on their own site please?
Comment #39
lotyrin CreditAttribution: lotyrin commented> Any futher problems should open new issues.
Please open new issues, include affected version information and include steps to reproduce.
Changes should be patches and should be able to pass review.