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.
Comment | File | Size | Author |
---|---|---|---|
#34 | drupal.menu-links-dynamic.34.patch | 15.12 KB | sun |
#32 | drupal.menu-links-dynamic.32.patch | 14.88 KB | sun |
#22 | dynamic_menu_links.patch | 14.88 KB | chx |
#20 | dynamic_menu_links.patch | 14.61 KB | chx |
#19 | dynamic_menu_links.patch | 15.52 KB | chx |
Comments
Comment #1
chx CreditAttribution: chx commentedNote: 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.
Comment #2
chx CreditAttribution: chx commentedSo, 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.
Comment #3
chx CreditAttribution: chx commentedWe agreed with pwolanin that this is a feature.
Comment #4
chx CreditAttribution: chx commentedComment #5
pwolanin CreditAttribution: pwolanin commentedagreed - 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.
Comment #6
chx CreditAttribution: chx commentedIf agreed then you crossposted me.
Comment #7
lilou CreditAttribution: lilou commentedPatch partially applied :
Comment #8
DamienMcKennaWould 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
Comment #9
chx CreditAttribution: chx commentedNo, 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_object
safely any time becausemenu_get_item
is now safeguarded from endless recursion.menu_get_object
from the new _to_arg handlers.menu_valid_path
becomes 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/%/edit
and check on it.Comment #11
chx CreditAttribution: chx commentedFixed 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.
Comment #13
chx CreditAttribution: chx commentedI have overcorrected so now forum tests pass and User language settings 55 passes, 0 fails, and 0 exceptions too.
Comment #14
chx CreditAttribution: chx commentedWith less debug. No other change. Thanks Dmitri.
Comment #15
stephthegeek CreditAttribution: stephthegeek commentedPlease 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.
Comment #17
chx CreditAttribution: chx commentedKeeping up with HEAD.
Comment #18
webchickFrom #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:
and the menu system handled the background stuff?
Comment #19
chx CreditAttribution: chx commentedThe problem is that while
user_uid_optional_to_arg
can happily work without having a$account
as an input, theto_arg
functions introduced here can not. What's more you need to runuser_uid_optional_to_arg
before you can load the current user, so it is not really possible to switch the order for that call. Also, the$map
that is available in the translate functions is the exploded version of the link path not the$map
of the router item.menu_get_object
gives you access to the$map
of the router item. So I propose the introduction of a wrapper aroundmenu_get_object
to significantly lower the amount of code to be copy-pasted.Comment #20
chx CreditAttribution: chx commentedHuh, I left in some cruft in the beginning, sorry.
Comment #22
chx CreditAttribution: chx commentedKeeping up with HEAD.
Comment #24
pwolanin CreditAttribution: pwolanin commentedI'm unlear from the whether a to_arg function is required or not for each % argument?
Comment #25
catchBumping 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).
Comment #26
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedwill this work in D6?
Comment #27
chx CreditAttribution: chx commentedThere is more , a lot more to this. We could, then, make tabs ordinary menu links themed as local tasks...
Comment #28
catchTabs 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.
Comment #29
drewish CreditAttribution: drewish commentedsubscribe
Comment #30
Damien Tournoud CreditAttribution: Damien Tournoud commentedLet's see if this one still applies.
Comment #32
sunRe-rolled against latest HEAD.
This indeed qualifies as critical bug. Tagging to get into my list of issues to push.
Comment #34
sunThis one will pass.
The concept needs work though.
Comment #35
mattyoung CreditAttribution: mattyoung commented.
Comment #37
q0rban CreditAttribution: q0rban commentedSubscribe
Comment #38
andypostSubscribe, patch outdated
Comment #39
andypostThis could easily solved with #320331: Turn custom_url_rewrite_inbound and custom_url_rewrite_outbound into hooks
Comment #40
fizk CreditAttribution: fizk commentedsubscribe
Comment #41
Damien Tournoud CreditAttribution: Damien Tournoud commented@andypost: those issues are pretty much unrelated.
Comment #42
chx CreditAttribution: chx commentedComment #43
pwolanin CreditAttribution: pwolanin commentedbump - this should still be a critical fix for D7
Comment #44
sun.core CreditAttribution: sun.core commentedComment #45
klonossubscribing...
Comment #46
Enemy CreditAttribution: Enemy commented#9: dynamic_menu_links.patch queued for re-testing.
Comment #47
sunRelated contrib module: http://drupal.org/project/menu_token
Comment #48
andypostRelated #144538: User logout is vulnerable to CSRF
Comment #49
andypostRefactoring of menu in #1463656: Add a Drupal kernel; leverage HttpFoundation and HttpKernel will make this obsolete
Comment #50
kscheirerSo is this issue now obsolete?
Comment #53
Fabianx CreditAttribution: Fabianx commentedThis deals with paths, which do not exist in D8 anymore => Back to 7.x