To replicate this, do the following:

1. Enable path_redirect and check the box to create redirects every time an alias changes.
2. Create a node with title "foo." Use the default alias ".../foo".
3. Edit the node again and change the path to ".../foo-bar". Leave the node title the same.
4. Observe that path_redirect creates a redirect from ".../foo" to the node at "/node/[nid]".
5. Click edit on the node. Do not save.
6. Observe that the redirect is gone!

What's happening is that, in the process of checking whether the current alias matches what would be generated automatically from the node title in the form_alter hook, the function pathauto_create_alias() calls _pathauto_alias_exists(), which detects the presence of path_redirect and then looks for any redirects to the node with from that URL and (!) deletes them.

Even even if we do want to be deleting redirects, surely we don't want to be doing it at edit time. My patch simply removes the deletion. As far as I can tell this has no ill effects other than having pathauto add numbers to a path when colliding with a redirect. If there is a better way to fix this, I would love to know about it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

semantric’s picture

Forgot to mention that path_redirect is version 6x-1.0-rc1 -- and it's probably a change in path_redirect behavior that brought this about (it didn't used to link to nodes directly but rather between paths).

Status: Needs review » Needs work

The last submitted patch, pathauto_redirect_deletion_fix_6x_2x_dev.patch, failed testing.

semantric’s picture

Status: Needs work » Needs review

second patch failure is because I didn't know about the auto testing ... obviously it is for a different version. Why the first one is failing so hard I don't know.

miccil’s picture

This patch work correctly for me on pathauto 6.x-1.5.
Will this fix included in new relases of 6x-1.x brach?

gooddesignusa’s picture

Is it safe to use pathauto_redirect_deletion_fix_6x_1_5.patch to patch pathauto 6.x-1.5 even though it failed testing?

gooddesignusa’s picture

I used pathauto_redirect_deletion_fix_6x_1_5.patch to patch pathauto 6.x-1.5 and it worked. I did some quick testing and everything seemed fine. If this is a patch to a pathauto file shouldn't this be inside the path auto issues?

omerida’s picture

Status: Needs review » Reviewed & tested by the community

The patch for 1.5 worked for me as well, without any loss of functionality. I found this issue trying to find why path_redirect_clear_cache(), which calls cache_clear_all(NULL, 'cache_page'), was being called so often on my site. Calling the clear cache when the node is being EDITED has the nasty side effect of clearing the cache once on displaying the form, and then the cache gets cleared AGAIN when node_save() is called.

roderik’s picture

Crosslink to D7 issue: #971552: Integration with Redirect Module Not Working (or not present)
where pathauto maintainers have removed the delete statement at comment #12 and are considering re-adding it from #13 onwards.
Stay tuned, I guess...

By the way: I haven't applied the above patch (yet?) because installing the pathauto_persist module seems to 'fix' the issue for me -- at least for the example mentioned in the first post.
(edit: indeed, pathauto_persist always prevents this thing from happening. It prefills $node->pathauto_perform_alias no node load, so pathauto_form_alter() never calls pathauto_create_alias() anymore. I assume this is the same on D7, but am not too fluent in D7 API yet.)

jessehs’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.37 KB

During the 1.5 patch's call to path_redirect_load_multiple, the language attribute of the redirect is not passed.

If the following patch is applied to the path_redirect module (that fixes language attribute for redirects): http://drupal.org/node/1834790
This caused some problems during i18nsync in the following scenario.

English (en) node/1 alias: "foobar"
German (de) node/2 alias "foobar"

German node/2 is updated to the path "foobar-baz"
A German redirect is automatically created for "foobar" >> node/2 where language = 'de'

During i18nsync, the English node/1's path is updated by pathauto.
But path_redirect_load_multiple detects that there is a redirect for the "foobar" path, even though that redirect has language = 'de'.
This results in node/1 path being updated to "foobar-0" and a redirect being created for "foobar" >> node/1 with language = 'en'.

The attached patch adds a language condition to the query. Rolled against the 1.6 stable release.

jessehs’s picture

Version: 6.x-1.5 » 6.x-1.6