| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | menu system |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (works as designed) |
Issue Summary
I need to have menu items that ether link to external sites, have a query string (GET variables) or named anchors. But I had problems with this because user defined menu items were being url encoded and this would break the links. So first I tried hacking the drupal_urlencode function using the parse_url function to try and make the drupal_urlencode function more flexible, but because this function is used more generally I had a problem with getting it to work under all conditions.
So I tried hacking the menu system and menu module, and I think I got it write.
before:href="node/123%3Fvar1%3Done%2526var2%3Dtwo%2523section1"
after:href="node/123&var1=one&var2=two#section1"
Not a 100% (&) but the links seem to work.
I searched drupal.org and found quite a few people also having problems with urls being incorrectly encoded.
http://drupal.org/node/85331
http://drupal.org/node/72812
http://drupal.org/node/59674
http://drupal.org/node/56755
http://drupal.org/node/76034
I think this is pretty important since users cant define any url they want as a menu item. And it is something I personally often require. I am not sure if this is a bug or feature request, but since the links were broken I thought it would be considered a bug.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| menu_paths_nax.patch | 3.52 KB | Ignored: Check issue status. | None | None |
Comments
#1
#2
How do I apply this patch to my currently running setup?
It doesn't seem to work in the same way as when I normally apply patches.
Thanks
jacauc
#3
This patch is for drupal CVS not for 4.7
I tested it using
patch -p0 < menu_paths_nax.patchand it worked. How do you apply patches.#4
#5
Brilliant. I was just about to post an issue about this.
I was going to work on a patch as well, but then I saw that chx is working on a complete menu overhaul, so can't do anything until that's done.
One comment, though: I found the linking system to be inconsistent. Menu links have one syntax, links from hook_link() and hook_alter_link() another, and breadcrumbs yet again are different. It would be great if we could use the same syntax for all three (the syntax for hook_link() seems the best).
#6
Note that the following patch does a lot to simplify links, and introduces a structure that should be reused by other modules: http://drupal.org/node/111347#comment-191811
Waiting for a reroll for HEAD...
#7
My strings REALLY SUCK. But, this feature is very easy to implement in the new system!
#8
My strings REALLY SUCK. But, this feature is very easy to implement in the new system!
#9
Possibly put these in a collapsed fieldset ("Advanced?")?
#10
I asked grugnog whether fragment or anchor and he basically said why separate fields. Right.
#11
I asked grugnog whether fragment or anchor and he basically said why separate fields. Right.
#12
Tested and menus save and edit, validation still works on invalid paths and the query/anchor is added back in correctly on edit/view. Looks good.
#13
Hm, how does the UI look like (screenshot) for this? I wonder how people provide query strings and anchors, which need to be derived differently from the actual site URLs on different sites. I see it is easy to split and to join strings here, but what is the user experience?
#14
Looks to me like there is no UI impact at all. We just use any querystring and fragment that is provided in the path field. i think thats reasonable, and even expected.
#15
OK, committed.
#16
#17
Any chance backporting this to 5.x?
#18
I've tested this on HEAD. Menu paths of the form 'path/to/node#fragment' work. But one might like to add a link to a local anchor on the current page, e.g. use '#fragment' as a menu path. This does not work on HEAD.
A practical example where this would be useful is "Skip Navigation" links for accessibility.
-- telcontar
#19
seems reasonable and arguably a bug with the original patch, so setting back to CNW.
#20
I second the backport to 5.x
#21
#22
please backport to 5.6/5.7. this has been very frustrating!
tx
#23
I'm glad I'm not the only confused party here.
http://www.mychildcanbehave.com
#24
I'm dubious about the anchor-only links - making those work is going to be very theme dependent. You could also add such a "skip" link to your template, or use hook_translated_menu_link_alter.
#25
+1 on backporting to a drupal 5.x version - would this still be possible or has it been done?
cheers,
tobias