Hi,
I tried to make an implementation of the hook pathauto_clean_alias in my module, unfortunately I was unable to find any documentation related to this hook. So, I took a look at the code and found the place where it's called (line 264 in pathauto.inc).
module_invoke_all('pathauto_clean_alias', $output);
This suggests that $output is passed as reference and my implementation should therefore look as follows.
function mymodule_pathauto_clean_alias(&$alias) {
if(something) $alias = 'something else';
}
But now I get the following error message (PHP 5.3):
warning: Parameter 1 to virtual_sites_pathauto_clean_alias() expected to be a reference, value given in /bla/includes/module.inc on line 483.
So, passing a string as reference is somehow not allowed... Obviously the following code does not work either.
function mymodule_pathauto_clean_alias($alias) {
if(something) return 'something else';
else return $alias;
}
This seems way cleaner, so I've changed the call of hook_pathauto_clean_alias (at line 264 in pathauto.inc) as follows.
foreach (module_implements('pathauto_clean_alias') as $name) {
$function = $name .'_pathauto_clean_alias';
$output = $function($output);
}
Since the hook is undocumented I have no clue whether my implementation should modify the alias as a reference or should return the modified alias. Hence I have no idea whether this is an actual bug, something related to the changed regarding references in PHP5.3 or my own stupidity.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | pathauto_clean_alias_fix.patch | 701 bytes | quicksketch |
Comments
Comment #1
gregglesI believe this is really about PHP 5.3. I'm not sure that the problem has really been "fixed" in core but we should do whatever core does to fix this.
Comment #2
quicksketchThis isn't a PHP 5.3 problem, it's just that module_invoke_all() NEVER passes anything by reference (that's why node api has that crazy node_invoke() function). I second Robbert's suggestion and believe that this hook should probably use a return value anyway. Though we could change it to by reference if that's preferred. Right now this hook doesn't do anything at all though, since you can't modify the variable by reference and pathauto doesn't do anything with the return values.
Comment #3
liquidcms commentedlol.. nice to know i came up with pretty much the exact same patch: http://drupal.org/node/212208
so i 2nd that this get committed as it does nothing as it stands now.. :)
Comment #4
gregglesThanks, guys!
Committed to 6.x-1.x, 6.x-2.x, 7.x-1.x:
http://drupal.org/cvs?commit=371906
http://drupal.org/cvs?commit=371908
http://drupal.org/cvs?commit=371910
Comment #5
quicksketchThanks greggles!