We have an issue which i'm sure is a rather small use case but annoying none the less. I believe it to be an issue with subpathauto_url_inbound_alter() not checking if the current path has a redirect set if someone is using the redirect module.
Our specific use case:
Term 1 alias: foo/bar
Term 2 alias: foo/bar/foo
Changing foo/bar/foo to foo/bar/foo2 creates a redirect on the term from foo/bar/foo to foo/bar/foo2. This is correct. Actually going to foo/bar/foo however, goes to the foo/bar term.
This is because the module is not checking to see if foo/bar/foo is a redirect before doing its thing and is finding a match on foo/bar once the trailing foo has been removed. Due to it finding a match, it never does the redirect.
Putting a check in subpathauto_url_inbound_alter() would be simple enough but would like to know what your opinion is on this is.
Comment | File | Size | Author |
---|---|---|---|
#18 | check_for_redirect-1850832-18.patch | 897 bytes | lolandese |
| |||
#14 | subpathauto_1850832_14.patch | 870 bytes | nord102 |
| |||
#10 | subpathauto-subpathauto_lookup_subpath-1850832-9.patch | 925 bytes | nord102 |
Comments
Comment #1
jarrodirwin CreditAttribution: jarrodirwin commentedAs we need a fix for this in our system asap, I have added a simple patch that checks if the redirect module is enabled then checks for a redirect before looking for a match on other parts of the URL.
The more I think about this the more the issue may be with how/when the redirect module kicks in and does the redirect on their end.
Anyway, patch attached.
Comment #3
Pls CreditAttribution: Pls commentedPatch works for me so marking needs review.
Comment #4
deanflory CreditAttribution: deanflory as a volunteer commentedThis issue is 3.5 years old and the latest patch now fails testing. Is this still an issue?
Comment #5
klidifia CreditAttribution: klidifia as a volunteer commentedYes it's still an issue and the patch does not deal with it adequately, for example:
/some/path-with/a-redirect/ that redirects somewhere else, e.g. /some/path-with/a-redirect-done/
User accesses path: /some/path-with/a-redirect/foo/bar
Ideally what would happen is the module would pick up that the initial redirect and the end URI would be:
/some/path-with/a-redirect-done/foo/bar
Comment #6
pub497 CreditAttribution: pub497 commentedThis is still an issue with the latest version, instead of taking to page not found it will just take you to the parent page.
ie /node/parent/childnotfound will take you to node/parent, which is pretty bad and misleading
Comment #7
frrrt CreditAttribution: frrrt commentedI just fixed this issue for our site and attached a patch with our solution. I'm new to uploading patches so I'd appreciate any help! :-)
It should do exactly what klidifia and nitesoul were mentioning:
If there exist a redirect /some/path-with/a-redirect/ to /some/path-with/a-redirect-done/
then /some/path-with/a-redirect/foo/bar will be redirected to /some/path-with/a-redirect-done/foo/bar.
Comment #8
nord102After applying the patch in #7, it did not fix my issue. I was experiencing the same issue as #6
Essentially the issue was happening when the moduel was assigning the $sourcein subpathauto_lookup_subpath. It was confirming that an alias for the $base_path existed and then was appending the $path_suffix to $source_based_path which would result in something like:
which would just return the node (node/12345, in the example) and ignore the appended $path_suffix.
Therefore in the attached patch, I changed how $source was being defined by using the $base_path instead of the $source_based_path and then checked that with drupal_valid_path() to see if it was valid.
Comment #10
nord102My mistake, this patch should work. (forgot to apply the previous patch first)
Comment #11
lolandese CreditAttribution: lolandese at HCL Technologies Limited commentedComment #12
mauritsl CreditAttribution: mauritsl commentedComment #14
nord102Re-uploading patch from #8 and #10. See patch description in #8
Comment #15
nord102Comment #16
lolandese CreditAttribution: lolandese at HCL Technologies Limited commentedThe OP describes the issue for taxonomy terms. It should be noted the same happens for nodes. Just to give some steps again to reproduce the issue (before applying the patch):
A redirect should now automatically be created from the old alias to the new alias (old/alias -> foo/bar/new). However before applying the patch it will be node/123/old/alias -> foo/bar/new. After applying the patch repeating the same steps the aliases will now be as expected thus it solves the issue.
Please commit.
Comment #17
lolandese CreditAttribution: lolandese at HCL Technologies Limited commentedOn second test it seems to disrupt the basic functionality. foo/bar/new/edit now throws a 404.
To see if we can adjust the conditional to only kick-in when needed.
Comment #18
lolandese CreditAttribution: lolandese at HCL Technologies Limited commentedWith the Redirect module enabled, the patch:
Comment #19
lolandese CreditAttribution: lolandese at HCL Technologies Limited commentedAttached patch solves the specific use case described by the OP when changing aliases (automatically generated redirect from old to new). You might still run into issues if manually defining the URLs through admin/config/search/path/add when an alias already exists for part of the newly created redirect (in the field Existing system path).