Good catch. However, I'm using this in conjunction with Persistent URL (the D7 version is available here: http://drupal.org/node/1070222), and I still get redirect loops. If I make Global Redirect lighter than PURL, then I don't get any redirect loops anymore. But the PURL rewriting doesn't work in this case.

Yeap, that is correct for D7, but since in D6 custom_url_rewrite_outbound wasn't a hook (bump) there was no way for module to alter URL's in a consistent way, there was no hook for it, so one had to be cerated. In D7 we have HOOK_url_outbound which should solve the problem.

BTW, you might want to check the patch in #346911-85: Redirect Loop and custom_url_rewrite ignored it was a cleaner version.
Anyway, if you check my patch, you'll see that I was calling the function I created in 3 places, which is where we need to get the clean verison of the url. I have no installation of PURL anywhere nearby to thoroughly test, but I'm pretty sure that if you modify the D7 version to call HOOK_url_outbound where I was calling my PURL fix function, it should work.

Question: How do I go about testing this? I'm considering adding a submodule to GR which is only used for simpletests (one which implements the appropriate hook inbound/outbound alters, for example). I have the beginnings of one here: #774950: Incompatible with hook_url_inbound_alter().

Err, why? From the discussion above it was already shown that the patch does absolutely nothing for drupal 7. It was intended for drupal 6, I think. Confusing also for me, since this issue is filed against d7.

I doubt registering the url_outbound hook should be done in the globalredirect module (dedicated module preferably), but that is a side note.

Committing this to the d7 branch is just wrong IMHO, see my previous reply.

It still needs some work to fix the problems with PURL, but if someone tries what I mention in number #5 I think it would fix purl problem.

Just to make sure we are all in the same page:

In d6 there was no hook to modify urls, but there was the posibility for developers to implement custom_url_rewrite_outbound, but it could only be used once across the whole site so modules couldn't implement it.

The D6 version of the module was calling custom_url_rewrite_outbound() which was fine.

The D7 version was still calling it, which was wrong, there is now a hook in D7 hook_url_outbound_alter which is what we need to invoke now. My patch fixes that. It removes the call to custom_url_rewrite_outbound which is unnecessary and it invokes the hook.

The HOOK is being called the way it should be called, the same way url() does

The patch I link to in #5 and #3 fix the redirect problem in D6, but they do it in a really hakish way, BECAUSE there was no hook to do it in D6.

In D7 the hook should, in theory, be implemented in PURL and any other module that modifies the url, and if called by Global Redirect, redirect issues should be fixed.

@Kiphaas7 Did you actually read the patch? And the issue? My patch implements drupal_alter('url_outbound') in the same way url() does. And it does fixes a problem. Also, correct me if I'm wrong, but registering a hook is not the same as invoking it. Here I'm not registering it, I'm invoking it.

I agree with jm.federico - Invoking a hook (either via drupal_alter, module_invoke or module_invoke_all) is not the same as defining one.

The patch seems to address several issues.

The suggestion which catch made in #774950: Incompatible with hook_url_inbound_alter() needs to be considered on two points: 1) it's for inbound, not outbound (two different ways of altering links) and (2) it "disables" GR if it thinks a URL has been altered. Ideally we don't want that.

I'm gonna mark this as fixed (as the patch does what it says on the tin). It'd be great if we could cover any specific issues in separate tasks.

It's very strange to me that globalredirect calls url_outbound on the request path... The request path should be the literal request path, shouldn't it? Outbound alter should be called to determine the page's alias, not to change the incoming/request path.