Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I pointed out some trouble I was having with project_release tokens to greggles in IRC, and he called it a pathauto bug. ;) pathauto_cleanstring() is only called on tokens, not the finished alias. So, if you have a trailing separator (e.g. from using a token that only conditionally has a value), the trailing separator is left in the resulting alias if that token happens to be empty for a given node.
Patch coming soon to add pathauto_clean_alias(), which (at greggles's scope-creep request invokes hook_pathauto_clean_alias(), too). ;) Stay tuned.
Comment | File | Size | Author |
---|---|---|---|
#30 | 545216-pathauto-fix-cleanalias.patch | 15.13 KB | Dave Reid |
#29 | 545216-pathauto-fix-cleanalias.patch | 16.33 KB | Dave Reid |
#28 | 545216-pathauto-fix-cleanalias.patch | 14.19 KB | Dave Reid |
#23 | 545216-pathauto-fix-cleanalias-D7.patch | 12.16 KB | Dave Reid |
#17 | 545216-pathauto-fix-cleanalias-D7.patch | 11.27 KB | Dave Reid |
Comments
Comment #1
dwwNote, since both pathauto_cleanstring() and pathauto_clean_alias() want to strip leading/trailing separators and collapse duplicates, I moved that code into a private helper function that's shared by both.
Also note that pathauto_cleanstring() should be renamed to pathauto_clean_string(), but that's out of scope for this issue. ;)
Comment #2
gregglesI'm inclined to commit this but leave it here for review for a few days.
Regarding the new hook and why scope creep is ok: It will allow us to say "oh, you want to do crazy? yeah, do it yourself based on hooks." Instead of just arguing over what we are/aren't going to do.
Comment #3
dwwI'm all in favor of hooks. I just didn't want anyone to accuse me of slipping in a new feature with my bug unprovoked. ;)
Comment #4
LinL CreditAttribution: LinL commentedHi,
I'd run into a similar issue and was about to add a feature request/patch.
My alias pattern is something like this:
1/[field_name-raw]-[field_optional1-raw]-[field_optional2-raw]
I had been putting whitespace in the pattern, instead of the separator, and then had a small patch to pathauto.inc to trim and replace whitespace in the complete alias.
Now I've tested the patch in #1 instead and it works well for me. :)
Comment #5
gregglesThanks dww for this patch.
Applied to 6.x-1.x http://drupal.org/cvs?commit=327030
@dww Any chance you can port this to 7.x?
Comment #6
dwwThanks for committing! Sorry, no chance for me to port this anytime in the near future, no. :( My plate is already completely overflowing...
Comment #7
Dave ReidD7 patch attached for review.
Comment #8
Dave ReidRevised and committed to 7.x-1.x and 6.x-2.x as well.
Something I'm not exactly sure on is why the lowercasing is in a function meant to remove separators. Also, we need to document this in pathauto.api.php.
Comment #9
Dave ReidThis inconsistency needs to be fixed before the next release on any branch.
Comment #10
Dave ReidI'm really not happy with the hook that was added here.
Comment #12
Dave ReidComment #13
Dave ReidComment #15
Dave ReidComment #16
Dave ReidComment #17
Dave ReidWith a test creating a parent term with the alias 'My Crazy/Alias' and a child term with the pattern [term:parent:path]/[term:name], so it would end up as 'My Crazy/Alias/child-term'
Comment #19
Dave ReidGreat, we found a core bug: #870528: taxonomy_get_parents(), taxonomy_get_children(), and taxonomy_get_tree() do not return a full term objects.
Comment #20
Dave ReidStill hoping that core bug gets fixed shortly, but don't think I can do much until then. :/
Comment #21
Dave Reid#17: 545216-pathauto-fix-cleanalias-D7.patch queued for re-testing.
Comment #23
Dave ReidLet's try that without the debugging line and a stupid logic error in the test...
Comment #25
Dave Reid#23: 545216-pathauto-fix-cleanalias-D7.patch queued for re-testing.
Comment #26
Dave ReidDid a contrib-wide search for implementations of hook_pathauto_clean_alias and found none. Committing shortly.
Comment #27
Dave ReidCommitted to 7.x-1.x CVS. http://drupal.org/cvs?commit=404100
Comment #28
Dave ReidComment #29
Dave ReidTry this again with PHP 5.3 fixes...
Comment #30
Dave ReidComment #31
Dave ReidSimpletest for D6 is getting really irritating with not installing dependencies. This passes absolutely fine locally.
Committed to 6.x-2.x: http://drupal.org/cvs?commit=404400
Comment #32
Dave ReidCommitted to 6.x-1.x as well after manual testing: http://drupal.org/cvs?commit=404428
Comment #34
cyberderf CreditAttribution: cyberderf commentedSorted ?