Posted by chx on December 27, 2007 at 2:17am
24 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | menu system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | API clean-up |
Issue Summary
While menu_valid_path definitely shows we wanted to allow dynamic paths, this does not work (thanks to cwgordon7 who discovered they do not work though Charlie did not know that we intended them to work). Well, it's just a couple of to_arg functions missing and some checks against the global $menu_admin. After that I could vastly simplify menu_valid_path. Also, passing in TRUE as load argument did not work.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| dynamic_to_arg.patch | 10.77 KB | Idle | FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch dynamic_to_arg.patch. | View details | Re-test |
Comments
#1
Note: another patch is to rip out those lines of menu_valid_path for D6 and deem this a feature request for D7. I am waiting for a decision on this and then I will add texts: doxygen and some user text in the description on the menu item add/edit screen to let the user know about these paths.
#2
So, the patch allows you to enter "edit current node" which is only active on node pages into the menu for example. All this is only needed if want that, "edit my account" needs much less. However, the latter was broken in the original patch, now "user/%/edit" behaves the same as "user/%" which can be quite expected.
#3
We agreed with pwolanin that this is a feature.
#4
#5
agreed - I recall the intent for D6 was to allow the editing (or addition, I guess) of items in the Navigaiton menu like user/% that always have a valid return from the _to_arg.
#6
If agreed then you crossposted me.
#7
Patch partially applied :
--- Successfully Patched ---modules\aggregator\aggregator.module
modules\contact\contact.module
modules\filter\filter.module
modules\forum\forum.module
modules\node\node.module
modules\taxonomy\taxonomy.module
--- Failed ---
includes\menu.inc (Cannot apply hunk @@ 7 )
modules\menu\menu.admin.inc (Cannot apply hunk @@ 7 )
modules\menu\menu.module (Cannot apply hunk @@ 6 )
modules\user\user.module (Cannot apply hunk @@ 6 )
#8
Would this allow the following scenario?
* Create a taxonomy vocabulary "product"
* Create the terms "cats", "dogs", "elephants" in the "product" taxonomy
* Create a view page named "product/%" that accepts an argument of a term from the "product" taxonomy
* Create a menu item with the link_path "products/cats"
Currently in D6 this is not possible, and is a major limitation to the flexibility of all modules that can create custom pages, e.g. Views, Panels, etc.
Damien
#9
No, that's a whole another issue. This one is about the ability to add links pointing 'node/%/edit'. Edit: and similar dynamic paths. It adds a few cleanups/fixes:
menu_get_objectsafely any time becausemenu_get_itemis now safeguarded from endless recursion.menu_get_objectfrom the new _to_arg handlers.menu_valid_pathbecomes simpler because pointing to existing dynamic paths is simply allowed. This cleanup unlocks #190867: Remove access check for anonymous users when creating aliases btw.node/%/editand check on it.#10
The last submitted patch failed testing.
#11
Fixed two new to_arg functions to use objects instead of arrays. I rolled #372914: Link titles are broken when using a non-t callback in this to fix blog. Just wasted an hour to figure out I am biten by that bug again, it'd been much easier if it'd been in.
#12
The last submitted patch failed testing.
#13
I have overcorrected so now forum tests pass and User language settings 55 passes, 0 fails, and 0 exceptions too.
#14
With less debug. No other change. Thanks Dmitri.
#15
Please please please!
This is one of the questions I see most often on IRC/forums. It would make user profiles more linkable, for example if you're encouraging a user to fill out a certain section of their profile and need to have their UID in the path. Also an issue I see often with Ubercart -- you can't link to user/[uid]/files to send a user directly to their purchased files tab, order history, invoices, etc.
#16
The last submitted patch failed testing.
#17
Keeping up with HEAD.
#18
From #drupal:
The thing I don't like about it is that it seems like *every* module that defines a %something arg needs to define a function like aggregator_feed_to_arg(). and if one of them doesn't, then the behaviour people get is inconsistent.
The only difference between menu_to_arg and node_to_arg is how you're doing menu_get_object and what $arg is returned. That seems like a lot of copy/pasting for module developers for two lines of code. We're also exposing menu system internals like $map and $index.
What if these functions were something like:
<?phpfunction node_to_arg($node) {
if ($node) {
return $node->nid;
}
}
?>
and the menu system handled the background stuff?
#19
The problem is that while
user_uid_optional_to_argcan happily work without having a$accountas an input, theto_argfunctions introduced here can not. What's more you need to runuser_uid_optional_to_argbefore you can load the current user, so it is not really possible to switch the order for that call. Also, the$mapthat is available in the translate functions is the exploded version of the link path not the$mapof the router item.menu_get_objectgives you access to the$mapof the router item. So I propose the introduction of a wrapper aroundmenu_get_objectto significantly lower the amount of code to be copy-pasted.#20
Huh, I left in some cruft in the beginning, sorry.
#21
The last submitted patch failed testing.
#22
Keeping up with HEAD.
#23
The last submitted patch failed testing.
#24
I'm unlear from the whether a to_arg function is required or not for each % argument?
#25
Bumping priority. Not being able to link to valid paths is a bug, and it's causing complaints at #308263: Allow privileged users to bypass the validation of menu items (which I downgraded).
#26
will this work in D6?
#27
There is more , a lot more to this. We could, then, make tabs ordinary menu links themed as local tasks...
#28
Tabs as menu links would make things like og_panels a lot easier, and means you could do things like add a views argument as a tab.
#29
subscribe
#30
Let's see if this one still applies.
#31
The last submitted patch failed testing.
#32
Re-rolled against latest HEAD.
This indeed qualifies as critical bug. Tagging to get into my list of issues to push.
#33
The last submitted patch failed testing.
#34
This one will pass.
The concept needs work though.
#35
.
#36
The last submitted patch failed testing.
#37
Subscribe
#38
Subscribe, patch outdated
#39
This could easily solved with #320331: Turn custom_url_rewrite_inbound and custom_url_rewrite_outbound into hooks
#40
subscribe
#41
@andypost: those issues are pretty much unrelated.
#42
#43
bump - this should still be a critical fix for D7
#44
#45
subscribing...
#46
#9: dynamic_menu_links.patch queued for re-testing.
#47
Related contrib module: http://drupal.org/project/menu_token
#48
Related #144538: User logout is vulnerable to CSRF
#49
Refactoring of menu in #1463656: Add a Drupal kernel; leverage HttpFoundation and HttpKernel will make this obsolete