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.
With this rather simple patch (add 4 lines, removes 1) if I have url/alias for node/11 then url/alias/edit will go to node/11/edit which is cool. However, test/alias/edit won't display the view | edit tabs. This is uncool.
Comment | File | Size | Author |
---|---|---|---|
#16 | path_alias_arg.patch | 820 bytes | chx |
#15 | path_alias_arg.patch | 818 bytes | chx |
#12 | extend_path.patch | 1.58 KB | michaelfavia |
#11 | move_delete_from_edit.patch | 5.2 KB | michaelfavia |
#8 | path_alias_arg.patch | 738 bytes | chx |
Comments
Comment #1
Richard Archer CreditAttribution: Richard Archer commentednode.module inserts a callback for node/11/edit in node_menu() and without that in place the local tasks won't be created. The backtrace for the call to node_menu() is:
As you can see, the callbacks are inserted from within menu_get_menu(). Which is called before you expand the alias.
The attached patch parses the query string and replaces the first (i.e. longest) alias it finds. It does this before the menu is built, so modules have access to GET['q'] containing the internal path and callbacks can then be registered properly.
It's not quite as elegant as yours, requiring the $path to be parsed twice. But this is only done once per page view so the performance hit should be minor.
Can this break anything? All it is doing is converting GET['q'] to what it would be if URL aliases were not enabled.
Comment #2
chx CreditAttribution: chx commentedThanks Richard, works nicely! The impact is indeed negligeble as practically all paths have only less than 4 slashes in it, most have only one, so the plus code run is almost nil.
If this gets in, I'll introduce functionality in taxonomy_term_page (it's simple, again) which will make possible paths like tags/tagname . How cool is that? :)
I hope this is not too late for 4.7 -- we are changing no public APIs.
Comment #3
Dries CreditAttribution: Dries commentedGoing to put this on hold for 4.8.
Comment #4
Richard Archer CreditAttribution: Richard Archer commentedI agree... there needs to be changes right throughout core to take advantage of this feature. Best to hold off.
Comment #5
eaton CreditAttribution: eaton commentedI'd like to toss in my vote for 'put it in, this hurts nothing and helps a lot.' Modules that accept parameterized urls are essentially un-aliasable in drupal at present. Views.module in particular -- one of the most powerful tools since ecommerce, in my opinion -- would benefit greatly from this patch.
Is there any reason to keep this OUT of 4.7, other than minimizing changes for the beta?
Comment #6
lilou CreditAttribution: lilou commentedbumping postponed issue to HEAD.
Comment #7
chx CreditAttribution: chx commentedI am interested... Needs to be in path init. Quite easy...
Comment #8
chx CreditAttribution: chx commentedMostly Richard's code , moved in place.
Comment #10
gregglesI guess that there could be a performance issue from this patch due to the call to drupal_get_normal_path which eventually does a query. But, it's a query on a unique index so it should be quite fast.
Ever week there is about 1 support/feature request for this exact feature in the Pathauto issue queue. Adding this would be a feature worthy of mention in the CHANGELOG.txt in my opinion.
Comment #11
michaelfavia CreditAttribution: michaelfavia commentedReviewed patch 1 style fix for string concatenation and one unit test fix in the taxonomy module on a dangling / for a path on a drupal post. Will write additional unit tests for coverage of this if anyone has specific ones theyd like to see covered please speak up.
Comment #12
michaelfavia CreditAttribution: michaelfavia commentedCrap sorry, reviewing too many patches on my desktop. (that is a good thing right?!?!) Here is the patch for the issue please ignore the previous patch.
Comment #13
chx CreditAttribution: chx commentedLet's get the bot on it.
Comment #15
chx CreditAttribution: chx commentedYou were fixing symptoms instead of causes. Drupal should work with foo/bar/. So an explicit test for this is required. Another test that'd be nice to see is that if node/11 is aliased to foo then indeed foo/edit works as intended.
Comment #16
chx CreditAttribution: chx commentedMinus old code style violation.
Comment #18
tstoecklerIs the test you mention in #15 implemented? Because that exact functionality does not work for me, though it could be a local problem.
Comment #19
Dave ReidI like this feature. Neat!
Comment #20
catchVery nice, subscribing.
Comment #21
moshe weitzman CreditAttribution: moshe weitzman commentedLovely. I'd love an inline comment explaining WTF is going on.
Comment #22
ksenzeeThis is a lovely, lovely feature. I have to say it seems a little odd to query the database six times for a path like admin/build/views/edit/myview/somearg. I guess greggles is right in #10 that it's a query on a unique index so it shouldn't be much of a hit, but it just feels wrong. Would it make sense to instead pass an array through drupal_get_normal_path to drupal_lookup_path?
Comment #23
David_Rothstein CreditAttribution: David_Rothstein commentedGreat functionality!... but I agree that it would make more sense to put the code in drupal_lookup_path(). Not only would that open up the possibility of optimizing the database calls, but also, it would allow people who call functions like drupal_get_normal_path() and drupal_get_path_alias() to get this aliasing functionality also, which I assume is the desired behavior? (The way the patch works now, if you have an alias pointing from
my/url/alias
tonode/1
, then visitingmy/url/alias/edit
in the browser gives you the edit page, but clicking on the "edit" link itself still sends you tonode/1/edit
...)Also, although this feature is very useful, I can imagine situations where someone might want to turn it off. If I have an alias pointing from
my-amazing-site-dashboard
toadmin
, maybe I don't want every single path underneathadmin
to inherit that alias too? (If this turns out to be complicated, I definitely wouldn't want to see the patch held up for it, but it could be as simple as adding an extra column to the {url_alias} table. No need for core to provide a user interface for it, IMO.)Comment #24
moshe weitzman CreditAttribution: moshe weitzman commentedLooks like solving David's original request, that all outbound links have these nice aliases, is a major performance hit. It is also a prerequisite IMO. We need some fresh ideas on this one.
Comment #25
sunWorking, but different implementation: http://drupal.org/project/subpath_alias
Bumping to 8.x.
Comment #26
valthebaldDoes this belong to core anymore?
Comment #27
chx CreditAttribution: chx commented