Posted by berenddeboer on March 5, 2008 at 5:51am
| Project: | Pathauto |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
When you set "Update action" in the pathauto settings to "Do nothing" pathauto still overwrites any alias you've changed from the default to its own preference.
It should leave every alias alone when you set this, only set it when the alias is empty.
It would be even better if it only updated the alias if the previous alias was equal to the auto-generated one. If it was different, the user had changed it, so don't touch it, as the user is always right.
Comments
#1
On the first point - yes, the "Do nothing" option appears to be broken.
Regarding not aliasing if the user has manually edited the alias, this is a relatively complex problem that has been discussed #180440: If an alias is manually created, don't automatically replace it on edit elsewhere so let's just focus on the first problem here.
#2
Yes, the overwriting problem really exists. Just came across with it.
Scenario:
1. Page nodes are configured to generate alias "page/[title-raw]"
2. Create a Page, fill title, descr, leave alias empty, click Save. Alias is generated correctly.
3. Edit this page. The previously generated alias is the default value for alias field which is correct.
4. Click Edit. The old alias was removed and new one is not created.
#3
The overwriting problem is driving me craaaaazy today. So disabled the module.
Is a fix imminent?
#4
Not from me - I don't use that option and don't have time to work on random problems. When I plan to work on an issue I try to be sure to set the "Assigned" field.
#5
Sorry, did you mean this
or overwriting issue?
I'm sure such a request is quite a big shift for this module and it should be taken out of this issue. This is a new feature anyway.
But what about
Will you have time to fix it? If not I'll try to help you.
#6
In my opinion this issue is only about one thing - as all issues should only be about one thing ;) - and that one thing is this:
1. Install pathauto and set the update action to "Do Nothing"
2. Create a new page and see it get aliased
3. Edit that page
Expected results:
The alias stays the same
Actual results:
The alias is changed/deleted.
I'm not working on that specific problem.
#7
ARDAS,
Will you be looking into this? I'm certainly interested in having "Do Nothing" fixed. Just was curious where you're at before I jump in.
#8
Yes, I'll attend to this problem this week end and hope will provide the patch which solves this issue...
#9
Actually I took a little time today to trace it down. It was little more nested then I would have guessed. :?
At first I just patched pathauto_form_alter() such that if pathauto_update_action is 0 and there is already a path, then Autoupdate isn't checked.
That works fine for mode "0" and it stopped the overwriting. However other modes like "1 - Create a new alias. Leave the existing alias functioning" don't work either. So I moved on to pathauto_nodeapi and it's update calls.
After about an hour wondering why _pathauto_existing_alias_data($src) NEVER brings back any previously set paths, I found the answer:
Pathauto is installed with a module weight of 1, so it's always called after the core "Path" module. That is a problem as the core Path module deletes the existing alias before Pathauto checks for it.
I'm assuming there is a reason the Pathauto isntall has a weight of 1, but I think something is going to need to be changed there. Right now the autoupdate options are a mess with it running afterward.
I literally just found this a couple minutes ago, so I'm trying to get my head around things. But I figured I could at least point you in the right direction. I think :)
I'm going to continue looking into it as well, as I really need pathauto_update_action to work properly. Without it, some "superusers" on my sites will surely be trashing the beautiful fixed paths I have for some pages. :)
#10
The reason pathauto has to have a weight of 1 is so that paths that involve taxonomy tokens work. If pathauto is set to a weight of 0 then, for example, using [cat-raw] in a node path no longer works :( And that is a fairly popular token.
#11
Ahh... thanks. I guess tommorrow I'll have to dig into why exactly the Path module is deleting the exiting alias in the first place. Hopefully it's just something that can be fixed on the node edit form I guess. We'll see...
#12
I think the reason for Path module deleting any existing alias when the ceckbox "Automatic alias" is checked, is the javascript that disables the textbox where a custom alias (path) can be entered.
When "Automatic alias" is checked, the (disabled) path-value does not seem to be submitted, and I think that causes Path module to think that the user submitted an empty path (meaning "I want to remove the old alias").
I guess one solution could be to create a hidden value in pathauto_form_alter() where the old alias is saved, and use that at a later stage to prevent or restore the deletion. I'll test if the pathauto_nodeapi($op='presave') can be used. Having more alias than one may require extra caution.
To see that javascript has influcence: Disable javascript and set Pathauto's "Update action"="Do nothing". Edit a node and manually set an alias for it using the Path-textbox (or take one that has an alias). Then edit it again and check "Automatic alias". Since Do nothing is selected no new alias is created, and the old alias remains.
Now enable javascript and repeat. No new alias is created, but the old one is deleted.
I used the web browser Opera, perhaps the handling of a disabled textfield is browser-dependant?
#13
Yes, I think the idea in #12 worked. Credits to Moonshine for pointing out the right direction :-)
Test if the attached patch does it for you.
#14
I wish I would have visited here before loading up the editor earlier :-O I ended up with almost the exact same conclusion. (although I have to say your solution looks more elegant and consise). I didn't make any diff yet, so I'll give yours a shot and report back.
#15
Your patch works fine for me. :) I tested it with/without aliases and with pathauto_update_action = 0,1 & 2. Is pathauto_update_action = 3 a forthcoming feature? Or something removed perhaps? As it's not a visible option in the admin I didn't test it.
I have a couple other small changes that I think fit well with this patch. They concern the form display for pathauto_update_action = 0,1 & 2 and whether "Automatic alias" should actually make new alias for pathauto_update_action = 0.
This will be a small, easy to read patch. Should I include it as part of this issue or create a new one? Not sure what the maintainer would prefer. :)
#16
Thanks to both of your for testing and patch. I am not in a place to be doing much testing nor development so it is really useful to have folks helping out like this.
I believe that pathauto_update_action = 3 is used for path_redirect. Can you install that module and test it out as well?
@Moonshine - can you create that as a new issue? I am a big fan of keeping this as discrete as possible when it comes to issues and patches.
Thanks again!
#17
Path Redirect is pretty slick, I think it's a keeper. I installed and tested it with pathauto_update_action = 3 and everything works fine. (Redirects are added and can be viewed in the URL Redirects admin).
I think this patch is good to go. :)
I'll start up an issue for my UI tweaks tommorrow. Nothing revolutionary, but I think they make sense.
#18
I love this module and I actually think it should be part of the core - maybe it will be in ver 7.0. There is one thing and I'm not sure if it is a bug related to the last issue posted or if this is a new feature request.
When I first built my site I didn't really understand how this module worked so didn't make full use of it and just used the [title] for the path name. I am now making full use of content types and taxonomy to structure my site content so have learned how to add those into the path which makes this module so great!
Now I have a bunch of old paths that only contain [title] in the path and I want to add the new aliases and keep the old ones so as not to have too many 404 errors. This part works half right in that when selecting the 'Create a new alias. Leave the existing alias functioning.' it will create the new path and leave the old one in place. The problem is, the 'URL path' in the node still shows the old alias and the URL address of the node remains the old alias.
Is there a way to make the new alias the default one?
Hope this is an easy fix.
Thanks,
Shelley :)
#19
Nevermind - just found the answer by reading the Pathauto 5.x-1.x to 5.x-2.x New Features and Upgrade Guide (http://groups.drupal.org/node/6706)- sorry - i missed this...
#20
Hi sfinerty, I'm glad you are able to find the right documentation. In the future please be sure to create a new issue for questions if your question isn't directly related to the current one. While you question is somewhat related to this issue, it's not the exact same thing.
@emok and @Moonshine - thanks again for your help on this. I've committed it to the 6.x branch.
#21
I just wanted to say thank you for this patch. I have no idea how to apply patches but was able to parse it out and do it manually...it appears to work.
Here's what I'm seeing. can someone confirm that this is proper behavior?
1. I have pathauto set to "do nothing" when a node is updated.
2. I open a node with a custom path written for it.
3. The pathauto checkbox is checked. The text below reads my custom path.
4. I hit submit after making no changes.
5. My node still has the same path as it had before.
Does this sound right? I expected the checkbox to be turned off, but I really don't care so long as it works! I've got a staff of 20 people working with my site...and don't want any of them to ever screw up the URL to one of our stories.
#22
The patch is actually already in the latest dev version available here also.
What you are seeing is the "correct" behavior currently.
However I agree with you, I think that checkbox should just be *unselected* with "do nothing" set. (Rather then it just being ignored by the processing code that follows). I actually have a patch for this behaviour that I was going to turn in here for consideration, just haven't had the time.
I'll get it up here in the next couple days if you want to try it out...
#23
Hey Moonshine. I'd be happy to test if you wanted to post something up.
#24
Re: The post above I've added a patch that fixes the checkbox for mode = 0 and implements extended Update Actions. It may be more then most people need, and needs further testing, but it's at:
#242456: Allow unique "Update Action" settings per item type (node, taxonomy,etc) or item (eg. page, story, vocabulary, etc)
#25
Automatically closed -- issue fixed for two weeks with no activity.