Closed (fixed)
Project:
Pathauto
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
5 Mar 2008 at 05:51 UTC
Updated:
21 Apr 2008 at 21:02 UTC
Jump to comment: Most recent file
Comments
Comment #1
gregglesOn 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.
Comment #2
ardas commentedYes, 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.
Comment #3
buddaThe overwriting problem is driving me craaaaazy today. So disabled the module.
Is a fix imminent?
Comment #4
gregglesNot 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.
Comment #5
ardas commentedSorry, 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.
Comment #6
gregglesIn 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.
Comment #7
Moonshine commentedARDAS,
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.
Comment #8
ardas commentedYes, I'll attend to this problem this week end and hope will provide the patch which solves this issue...
Comment #9
Moonshine commentedActually 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. :)
Comment #10
gregglesThe 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.
Comment #11
Moonshine commentedAhh... 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...
Comment #12
emok commentedI 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?
Comment #13
emok commentedYes, 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.
Comment #14
Moonshine commentedI 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.
Comment #15
Moonshine commentedYour 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. :)
Comment #16
gregglesThanks 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!
Comment #17
Moonshine commentedPath 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.
Comment #18
sfinerty commentedI 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 :)
Comment #19
sfinerty commentedNevermind - 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...
Comment #20
gregglesHi 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.
Comment #21
bflora commentedI 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.
Comment #22
Moonshine commentedThe 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...
Comment #23
bflora commentedHey Moonshine. I'd be happy to test if you wanted to post something up.
Comment #24
Moonshine commentedRe: 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)
Comment #25
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.