Remove old_alias related code

We have ests that do exactly this I think, it seems to work there and no other user reported this. I also just tried and it works fine for me.

Check your other modules, any that could be related? Try disabling them and see if you can figure out if it is a specific module that is causing this.

Can't do much without being able to reproduce this.

How to reproduce:
- Create content type;
- Set path pattern for that content type;
- Create node;
- Save node;
- Edit node;
- Try to change path alias in node edit form;
- Save node;

New path alias should be created, but it is not.

A bug appeared at "pathauto_entity_presave".

Did you enable the checkbox to disable automatic generation?

I did exactly that and it worked fine for me, so that alone isn't it, there has to be something else.

I also use the "Lightning" distribution.
And also observed the bug.
And that changes in patch solve my problem.

> Did you enable the checkbox to disable automatic generation?

The checkbox should be unchecked for the ability to change generated alias to the new one.

Recreated patch for latest 8.x-1.x-dev version.

Thanks. I'm not sure I understand this yet, can we test this somehow? Meaning, come up with a failing test that passes with your change?

Here the updated PathautoNodeWebTest.
After some investigation, I suppose that it will be always "false positive" (line in test after the comment "// Make sure that manual alias really updated."), because "$entity->path->old_alias" always comes empty in "pathauto_entity_presave" while the test. But when user manually submit form "$entity->path->old_alias" becomes with correct "old_alias" value. Field "path[0][old_alias]" have "#type" equal "value", so it never render at page. I have no idea how to check "old_alias" in test for now.
But the real problem is the wrong expression in "if" construct.
Code should automatically restore ($entity->path->alias = $entity->path->old_alias;) "old_alias" when:
- checkbox "pathauto" checked (value = 1) compared with "PathautoState::CREATE" (value = 1, that mean "An automatic alias should be created") but not with "PathautoState::SKIP" (value = 0, that mean "An automatic alias should not be created.");
- and
- "old_alias" not empty.

So now when "pathauto" checkbox is unchecked then it trigger that "alias restoring" code.
With patch "alias restoring" code trigger when "pathauto" checkbox is checked and cover the situations when JavaScript is disabled.

@Berdir
I think it /might/ be a caching issue, see #2833414: PHP 7 compatibility with apc (comment #4), just a pointer, not entirely sure if it's related.

We ran into this, too.

We're using Lightning 8.x-2.00 and Pathauto 8.x-1.0-beta1. PHP 5.5.24.

The patch in #15 fixed it. Thanks!!

If I look at the code of Path and Pathauto correctly, the proposed patch should have no effect (at least without Lightning).

In core/modules/path/src/Plugin/Field/FieldWidget/PathWidget.php, we have:

$element['alias'] = array(
'#type' => 'textfield',
'#title' => $element['#title'],
'#default_value' => $path['alias'],
'#required' => $element['#required'],
'#maxlength' => 255,
'#description' => $this->t('Specify an alternative path by which this data can be accessed. For example, type "/about" when writing an about page.'),
);

I checked, and $path['alias'] contains the current URL alias, so #default_value is correctly populated.

In modules/pathauto/src/PathautoWidget.php, we have:

if ($entity->path->pathauto == PathautoState::CREATE && !empty($entity->path->old_alias) && empty($entity->path->alias)) {
$element['alias']['#default_value'] = $entity->path->old_alias;
$entity->path->alias = $entity->path->old_alias;
}

// For Pathauto to remember the old alias and prevent the Path module from
// deleting it when Pathauto wants to preserve it.
if (!empty($entity->path->alias)) {
$element['old_alias'] = array(
'#type' => 'value',
'#value' => $entity->path->alias,
);
}

But with and without the "automatic URL alias generation" checkbox checked, $entity->path->old_alias and $entity->path->alias are always empty, so we enter none of the two IF blocks.

Now in modules/pathauto/pathauto.module in pathauto_entity_presave():

if ($entity->path->pathauto == PathautoState::SKIP && $entity->path->old_alias != '') {

Here again, $entity->path->old_alias is always empty, and $entity->path->alias is always correctly populated before reaching this IF block, so we never enter this IF block, making the value of $entity->path->pathauto irrelevant.

Could someone tell me where $entity->path->old_alias is supposed to be defined? Is that old code from 7.x which could/should now be entirely removed? Am I misreading something?

Yes, can confirm everything that #23 says. This code there never worked and we have our own solution for this problem in \Drupal\pathauto\PathautoItem::postSave().

But *if* someone sets the alias, then this can happen indeed.

I'm changing this to a task to remove the old_alias related code, I'd expect that everything is still working fine now.

At @Berdir's request, I tested this manually in Lightning, which includes the latest Pathauto. We have most definitely seen this problem manifest itself, but we thought it was due to bugs in our own alias-handling code and tabled it for now. I'm relieved to find that it's not us, and I'm very pleased that this patch fixes the problem under manual testing.

Setting to major. Pathauto is broken on Lightning without this patch, for one thing; but more importantly, Pathauto will be broken without this patch once core starts auto-loading path aliases (#2846554: Make the PathItem field type actually computed and auto-load stored aliases).

Thanks for testing. Committed.

Thanks for the fix! I thought the issue was with Lightning's alias-related code, but it turns out the fix is here. Just wanted to link a corresponding issue in Lightning's queue (which I've marked as a duplicate): #2838358: Code in lightning_core_entity_load prevents manual override of paths.

Can someone push out a new release/dev with this in pretty please? It doesn't work in dev still... #afaict - the content type I have tried to set aliases on doesn't have a pattern even, but the alias won't save.

This is part of the latest release and your problem doesn't sound like it has anything to do with this, it's kind of the opposite in fact.

I'm *reaaally* sorry - I thought this was to do with the pathauto settings. I used to gen aliases for this content type. I tried a lot of things to prove what it was... Seemed to fit this at the time. I uninstalled pathauto and redirect, and it seems like it's something to do with how aliases and menus link together in core but still not quite worked out where the issue is. Ignore me, such a n00b today... :(