At least for me, the Drupal 7 version was not quite working.

I have Global Redirect installed in a fresh version of Drupal HEAD in a subdirectory of my localhost.

I was able to get basic functionality working with a few modifications, which I will post in a patch.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

I will post an explanation below.

Anonymous’s picture

+++ globalredirect.module	16 Feb 2010 13:07:22 -0000
@@ -64,7 +64,7 @@ function globalredirect_init() {
-      isset($_REQUEST['q']) &&
+      isset($_GET['q']) &&

The _REQUEST superglobal doesn't have a value for 'q', at least in my install. This means that the conditional was skipped altogether.

Changed to _GET['q'], which is used throughout core modules like poll and taxonomy and I think should work for all use cases?

+++ globalredirect.module	16 Feb 2010 13:07:22 -0000
@@ -87,20 +87,15 @@ function globalredirect_init() {
-      language_url_rewrite($path, $options);
-    }
-    $prefix = rtrim($options['prefix'], '/');

language_url_rewrite is not a function in D7. Thus, the prefix is not set correctly, which breaks functionality when Drupal is in a subdirectory.

I'm not sure how to get the language prefix, but it should just be appended to the basepath once it's retrieved?

+++ globalredirect.module	16 Feb 2010 13:07:22 -0000
@@ -129,9 +124,9 @@ function globalredirect_init() {
-    if ($prefix && $alias) {
+    /*if ($prefix && $alias) {
       $prefix .= '/';
-    }
+    }*/

Because I set the prefix to the base_path, there is already a slash at the end

+++ globalredirect.module	16 Feb 2010 13:07:22 -0000
@@ -143,9 +138,9 @@ function globalredirect_init() {
+      if (str_replace($prefix . $alias, '', $_SERVER['REDIRECT_URL']) != '') {

It seems like checking against _SERVER['REDIRECT_URL'] is perfect for this use case.

This does remove the redirect_slash check. I didn't look into what it was supposed to be doing, but in my basic setup what it was doing was defaulting to TRUE and creating a redirect loop.


Like I said above, this works in my very basic use case. I haven't tested it in others and know for sure that it will break in some.

Powered by Dreditor.

Anonymous’s picture

It seems like the $_REQUEST and $_SERVER superglobals are handled differently based on whether Clean URLs are turned on.

When Clean URLs enabled $_SERVER['REDIRECT_URI'] is set, $_REQUEST['q'] is not set.

When Clean URLs are disabled, the opposite is true.

There is a difference in the way the two are handled ($_SERVER['REDIRECT_URI'] contains the base_path).

Anonymous’s picture

I tested the current HEAD on both a recent install of MAMP and a recent install of the Acquia Stack. It didn't work on either when enabled, I'm not sure whether this is a server issue of a change from D6 to D7.

The patch I attached before didn't work when Clean URLs were not enabled and when accessing the front page (regardless of whether clean urls were enabled). This patch fixes that in the basic case, but ignores some of the other logic in the module so will not work in other cases.

Anonymous’s picture

Status: Active » Needs review
Dave Reid’s picture

I wonder if we should switch to using hook_url_inbound_alter() instead of hook_init()?

Anonymous’s picture

Status: Needs review » Needs work
FileSize
3.07 KB
6.31 KB

It looks like hook_url_inbound_alter should work nicely.

I have changed the patch to use that hook and also have added tests for the basic case. This still just fixes the basic case and ignores some of the other logic that was in the original.

I actually don't know how to add new files if I don't have write access to the repository, so I'm just posting the .test in another file... if anyone has done that before, instructions would be welcome :)