site_frontpage hack breaks language path prefixes

This patch works for me

This patch creates a conflict with overlay module. When I apply this patch, the overlay don't appears any more. O_o

I'm having problems with this patch. When applied, I see an error of constant undefined (http://skitch.com/robertgarrigos/r8ymu/cesca-cesca). If I define the constant within the hook_init function, where the constant is declared, I got an error of constant already declared (http://skitch.com/robertgarrigos/r8yq3/cesca-cesca).

including once the locale.inc file solves the problem. I attach a patch against beta2 with it.

Here is my version of solution - just try to remove language prefix from path because this is what's is creating a problem.

 $_GET['q'] = request_path();

global $language_url;
list ($language, $_GET['q']) = language_url_split_prefix($_GET['q'], array($language_url));

drupal_path_initialize(); 

Seems to work OK. I also raised priority to major because multilingual sections of Drupal site (with language prefix in url) will not be accessible because of this bug.

Updated patch from #4 - it seems that we need to check for language_url_split_prefix function first - it will not be defined if we do not have enabled other languages on site.

It works for me

strongarm-lang_pref-998070.patch from #5 works fine.
a.t.m., strongarm quite reliably breaks multilingual Drupal sites, so this need's a solution - and several are offered here.
so what's the status of this issue? Did or will any of the patches get committed?

strongarm-lang_pref-998070.patch from #5 works fine.

#5 works fine for me too.

This issue its related to #1062452: strongarm_set_conf() needs to be called sooner.
#1062452: strongarm_set_conf() needs to be called sooner fixes this issue and doesn't need this ugly hack in strongarm_init.

edit: fixed link to the related issue.

I think webflo mean #1062452: strongarm_set_conf() needs to be called sooner

#5 works like a charm

#5 uses array($language_url) as a list of enabled languages. shouldn't it rather use language_list() ?

As say in #10 you should use and test the patch in #1062452: strongarm_set_conf() needs to be called sooner not the one in #5.

Also tested the patch in #1062452, works no better.
I'll try to isolate what causes the issue.

Thanks webflo & gagarine. The patch from #1062452-3: strongarm_set_conf() needs to be called sooner worked for me. Shouldn't we mark this issue as duplicate and continue over there?

This is not necessarily a duplicate as it is not clear if #1062452: strongarm_set_conf() needs to be called sooner will be accepted to fix the issue.

While hook_stream_wrappers there works fine, it is still a hack and hook_boot would be better, but has other issues.

This issue fixes only the language issue, but fixes it in a consistent way with the current code.

Best Wishes,

Fabian

I've tested both patch in #5 and the one provided in #1062452: strongarm_set_conf() needs to be called sooner and both of them work. In the meantime It seems I didn't use the strongarm function that needs this "$_GET['q'] = request_path();". I comment this line the issue is also fixed.

This patch should be commited as it fixes a major issue and the second solution may take some time to be achieved.

The patch in #5 and works for me and the one in #1062452: strongarm_set_conf() needs to be called sooner #3 too.

But i can't make globalredirect to work with any of this patchs. I don't know if the problem is in strongarm or global redirect but they don't play well together. If you think the problem is in globalredirect perhaps we could open a new issue there... I'll wait for your comments before.

Thanks.-

At first (as I said in #16), the patch from #1062452-3: strongarm_set_conf() needs to be called sooner worked for me. Then for some reason the bug came back. I couldn't figure out why it came back, but I did make sure that I didn't accidentally revert the patch. After applying the patch from #5 in this issue, language prefixes work again. It seems to me that in some cases, running strongarm_set_conf() sooner isn't enough to fix this bug.

That said, @idflood raises a very good question. Why is the line $_GET['q'] = request_path(); there in the first place? Instead of fixing this by adding code, could we fix this by removing code? I'm setting the status back to 'needs work'.

We may be re-inventing the wheel here... I've looked back in the Strongarm commit history. It seems that breaking language prefixes has been a known risk for a long time. When the initial D7 version was released, the D6 version already took care of stripping language prefixes, but that was removed from D7. See the commit diff at http://drupalcode.org/project/strongarm.git/commitdiff/1a5edcb0ea5a927c5.... The current 6.x-2.x HEAD still removes language prefixes.

Hi,

I am setting this back to RTBC, because:

This patch fixes a known problem, which was introduced by a necessary hack. (As strongarm is running really late in the startup process by just running in _init).

If we can get this in, this will be good and showing the D6 had similar problems is just making this better.

#1062452: strongarm_set_conf() needs to be called sooner can be used for fixing this in a better way and work on other ways to fix it.

Why is the line $_GET['q'] = request_path(); there in the first place?

$_GET['q'] = request_path(); is there to be able to set the site_frontpage variable via strongarm.

Removing this now might break existing installations, so this is not an option.

Language prefixes were removed to let strongarm_set_conf be called again and again, but this is also not an issue here, because _init is just run once per page request. (as far as I understood)

Best Wishes,

Fabian

subbing

Hello,

Applied patch #5 and it seems ok, no problems detected so far.

Any news about integrating this solution in a new release ?

Thanks !

Wanjee

Shouldn't the code in patch #5 be calling drupal_multilingual() first to check if this extra step is actually necessary?
(http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal...)

Subscribe.

This patch takes the work done in #5 and applies the feedback from #25, for a slightly cleaner and self-documenting if statement.

Reroll to remove whitespace #facepalm

Due to the way StrongArm works, it is actually possible that drupal_multilingual() can return TRUE (as a result of a strongarmed "language_count" variable) but the Locale module be disabled which will cause a Fatal error: Call to undefined function language_url_split_prefix() in strongarm.module on line 25. :P

For this reason, I suggest we go with a combination of #25, and #5.

Subscribe.

+1

Subscribe.

For those of you subscribing, it would be helpful to get your feedback on the patch in #29. thanks.

I'm using patch in #5 (that's almost the same of #29) and it's works well, since I already know my site is multilanguage. I'm just waiting for a definitive solution to update Strongarm and remove the patch from my makefile. Also, I don't know exactly what will be the "definitive solution", maybe #29 is fine as one.

#29 works for me, thanks!

#29 works for me too.

Many thanks for your efforts! This has been plaguing me for a few days. :-)

+1 also for the patch in #29, on 2.0-beta2.

Marking as RTBC in the hopes it will be merged in.

Here is an update of the patch in #29 that can be successfully used with drush make 6.x-2.2 and strongarm-7.x-2.0-beta2

@sfyn: cool, I wasn't aware that drush make needs the git commit hashes in there in order for it to work.

It doesn't, but the way that 2.2 works is that it applys patches with the flag -p0 - use the dev version of drush_make and you can use the old patch.

Patch works, thanks !

Patch in #38 works, but doesn't work with globalredirect 7.x-1.3

This combination leads to the following browser error: Error 310 (net::ERR_TOO_MANY_REDIRECTS).

patch #38 re-rolled on latest 7.x-2.x-dev

#43 worked well on dev release for me

Can also confirm that #38 works on strongarm-7.x-2.0-beta2

Haven't tried with global redirect

subscribing

+1 (had "no page found" issue after enabling strongarm - patch in #43 did solve my problem)

I can confirm that strongleg fixes the problem I have been having, without the need for applying either of the two patches. Much obliged!!!

patch #43 works for me, subscribing

so strongleg should become a required module? haven't tested if it fixes my problem - but it would seem odd to knowingly send users into a bug.

It seems foolish to use a whole extra module to work around blatant bugs in StrongArm, but it seems that this is what we have to resort to due to lack of progress on StrongArm. :-\

Strongleg works for me.

Subscribing.

Strongleg works for me too.

Seems like if we remove all of the shenanigans required in hook_init() that this problem will go away too.

When we get #1094598: Performance issues from strongarm_init() in to 6 and 7 we can readdress this.

Seems that the strongleg fix also works in combination with global redirect

Does the patch in #32 of #1094598: Performance issues from strongarm_init() fix this issue as well? I think we were addressing the base cause there, while the patch here is just treating the symptom.

@febbraro Yes, #1094598 fixes the issue with locale + strongarm in my testing. Locale path prefixing now works again with strongarm 7.x-2.0-beta3. I think this one is good to close.

Sweet, thanks @walkah