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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Richard Archer’s picture

Status: Needs work » Needs review
FileSize
1 KB

node.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:

node_menu(false) file: unknown line: 0
call_user_func_array("node_menu", Array[1]) file: /home/mel01/public_html/drupal47/includes/module.inc line: 192
module_invoke_all("menu", false) file: /home/mel01/public_html/drupal47/includes/menu.inc line: 1144
_menu_append_contextual_items() file: /home/mel01/public_html/drupal47/includes/menu.inc line: 220
menu_get_menu() file: /home/mel01/public_html/drupal47/includes/menu.inc line: 333
menu_execute_active_handler() file: /home/mel01/public_html/drupal47/index.php line: 15

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.

chx’s picture

Status: Needs review » Reviewed & tested by the community

Thanks 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.

Dries’s picture

Going to put this on hold for 4.8.

Richard Archer’s picture

Status: Reviewed & tested by the community » Postponed

I agree... there needs to be changes right throughout core to take advantage of this feature. Best to hold off.

eaton’s picture

I'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?

lilou’s picture

Version: x.y.z » 7.x-dev

bumping postponed issue to HEAD.

chx’s picture

Component: menu system » base system
Status: Postponed » Needs work

I am interested... Needs to be in path init. Quite easy...

chx’s picture

Status: Needs work » Needs review
FileSize
738 bytes

Mostly Richard's code , moved in place.

Status: Needs review » Needs work

The last submitted patch failed testing.

greggles’s picture

I 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.

michaelfavia’s picture

Reviewed 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.

michaelfavia’s picture

FileSize
1.58 KB

Crap 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.

chx’s picture

Status: Needs work » Needs review

Let's get the bot on it.

Status: Needs review » Needs work

The last submitted patch failed testing.

chx’s picture

Status: Needs work » Needs review
FileSize
818 bytes

You 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.

chx’s picture

FileSize
820 bytes

Minus old code style violation.

tstoeckler’s picture

Is the test you mention in #15 implemented? Because that exact functionality does not work for me, though it could be a local problem.

Dave Reid’s picture

Issue tags: +url alias

I like this feature. Neat!

catch’s picture

Very nice, subscribing.

moshe weitzman’s picture

Lovely. I'd love an inline comment explaining WTF is going on.

ksenzee’s picture

This 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?

David_Rothstein’s picture

Status: Needs review » Needs work

Great 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 to node/1, then visiting my/url/alias/edit in the browser gives you the edit page, but clicking on the "edit" link itself still sends you to node/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 to admin, maybe I don't want every single path underneath admin 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.)

moshe weitzman’s picture

Looks 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.

sun’s picture

Version: 7.x-dev » 8.x-dev

Working, but different implementation: http://drupal.org/project/subpath_alias

Bumping to 8.x.

valthebald’s picture

Does this belong to core anymore?

chx’s picture

Assigned: chx » Unassigned
Issue summary: View changes

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.