Hi,
we've chosen "Create a new alias. Leave the existing alias functioning." as action when an aliased node is updated. Our default path pattern is [menupath-raw].

Now we have a node which has no menu entry, so the pathauto-generated alias is empty. Unfortunately, this makes pathauto call path_set_alias('node/nn', '', NULL). Pathauto intents to create a new alias without removing the existing, that's why $pid=NULL, but since the $alias parameter is the empty string, path_set_alias will *remove* an alias for the given src!

Thus, if you created an alias manually for a node, this alias will be deleted when saving the node and the pathauto pattern for this node returns an empy string. This is definitely wrong for "Leave the existing alias functioning." The patch corrects this behaviour.

I set this to critical because we accidentally removed about two dozens alias over two weeks before we realized it (as we had several aliases per page), and some of those were published as references :-(

Comments

mrfelton’s picture

several aliases per page? Shouldn't you be using redirects instead of aliases for this? I mean, if you have one page available at several different urls, search engines will kill you for 'duplicate content'. I think you should be using something like the path_redirect module to set up 301 redirects for your additional urls instead of using multiple aliases for one node.

Frank Steiner’s picture

If you move a whole subtree of a menu then we need to regenerate the the aliases to reflect the menu structure. However, we cannot delete the old ones because they might have been published somewhere else.
So instead of manually setting up dozens of redirects we just let pathauto regenerate the new aliases in one step. Everything more complicated is sth. I wouldn't ask our users to do (because they wouldn't) :-/

Anyway, for this bug it doesn't matter if using several aliases is good or not. It's a feature of drupal and pathauto should respect that a node can have several aliases and not just delete them, especially if it has a setting "create a new one, keep the old one" :-)

And the bug hits you even with only one alias per page...

Frank Steiner’s picture

Ah, I see that the path redirect module works together with pathauto! I will to look at this. However, this wouldn't help currently because pathauto always only deletes or redirects one arbitraty alias, see http://drupal.org/node/373841
As long as this is not fixed I don't see a way to transform all our duplicate aliases into redirections without doing it manually.

greggles’s picture

Status: Needs review » Needs work

I agree this is a bug and fairly critical. I'm clear on why we need the $dst !== '' test - I'm not as clear that we need the or condition. Can you explain the logic a bit more?

Also, it would be great if you could provide patches for at least the two 6.x branches (6.x-1.x, 6.x-2.x). For 6.x-2.x we've started using constants which helps make the code much more readable.

Frank Steiner’s picture

If you have any other setting but "Leave existing alias functioning" then deleting the alias is correct here I guess.

E.g. when the node gets its alias from the menu path and you remove the menu entry while editing a node, then pathauto generates an empty alias. And then I guess it's correct to delete the old one for any other but the "Leave existing functioning" because according to the pattern the new alias is the empty string.

About the 2.x branch, I will try to do this. I don't have -2.x installed so I will have to setup another server for this and get used to the new version. Give me some time, then I will try to come up with patches for both branches (for this and the "recreating existing aliases") one.

dave reid’s picture

Priority: Critical » Major
dave reid’s picture

Status: Needs work » Fixed

I think this is already fixed when we added empty-alias prevention.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.