Support fragment only links [#90570]

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.

Comment	File	Size	Author
#11	menu_fragment_query-90570-11.patch	2.04 KB	chx
#11
#10	menu_fragment_query-90570-10.patch	5.36 KB	chx
#10
#8	menu-fragment-query_0.patch	1.62 KB	bdragon
#8
#7	menu-fragment-query.patch	1.62 KB	bdragon
#7
	menu_paths_nax.patch	3.52 KB	NaX

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

chx CreditAttribution: chx commented 22 October 2006 at 06:06

Title:	complex menu items = broken urls	» Menu should support query and fragment in links
Category:	bug	» feature
Priority:	Critical	» Normal

Comment #2

jacauc CreditAttribution: jacauc commented 24 October 2006 at 06:45

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

Comment #3

NaX CreditAttribution: NaX commented 24 October 2006 at 09:52

This patch is for drupal CVS not for 4.7

I tested it using patch -p0 < menu_paths_nax.patch and it worked. How do you apply patches.

Comment #4

NaX CreditAttribution: NaX commented 28 January 2007 at 20:23

Version:	x.y.z	» 6.x-dev
Priority:	Normal	» Critical
Status:	Needs review	» Needs work

Comment #5

moonray CreditAttribution: moonray commented 5 February 2007 at 22:20

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

Comment #6

Steven CreditAttribution: Steven commented 6 February 2007 at 08:00

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

Comment #7

bdragon CreditAttribution: bdragon commented 11 September 2007 at 20:13

File	Size
menu-fragment-query.patch	1.62 KB

My strings REALLY SUCK. But, this feature is very easy to implement in the new system!

Comment #8

bdragon CreditAttribution: bdragon commented 11 September 2007 at 20:13

File	Size
menu-fragment-query_0.patch	1.62 KB

My strings REALLY SUCK. But, this feature is very easy to implement in the new system!

Comment #9

bdragon CreditAttribution: bdragon commented 11 September 2007 at 20:16

Possibly put these in a collapsed fieldset ("Advanced?")?

Comment #10

chx CreditAttribution: chx commented 20 September 2007 at 09:53

Title:	Menu should support query and fragment in links	» Menu does support query and fragment in links
Category:	feature	» bug
Priority:	Critical	» Normal

File	Size
menu_fragment_query-90570-10.patch	5.36 KB

I asked grugnog whether fragment or anchor and he basically said why separate fields. Right.

Comment #11

chx CreditAttribution: chx commented 20 September 2007 at 09:54

File	Size
menu_fragment_query-90570-11.patch	2.04 KB

I asked grugnog whether fragment or anchor and he basically said why separate fields. Right.

Comment #12

Owen Barton CreditAttribution: Owen Barton commented 20 September 2007 at 20:44

Status:

Needs work

» Reviewed & tested by the community

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.

Comment #13

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 25 September 2007 at 11:37

Status:

Reviewed & tested by the community

» Needs review

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?

Comment #14

moshe weitzman CreditAttribution: moshe weitzman commented 27 September 2007 at 02:33

Status:

Needs review

» Reviewed & tested by the community

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.

Comment #15

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 27 September 2007 at 17:00

Status:

Reviewed & tested by the community

» Fixed

OK, committed.

Comment #16

(not verified) CreditAttribution: commented 11 October 2007 at 17:12

Status:

Fixed

» Closed (fixed)

Comment #17

telcontar CreditAttribution: telcontar commented 12 October 2007 at 09:01

Any chance backporting this to 5.x?

Comment #18

telcontar CreditAttribution: telcontar commented 14 October 2007 at 09:19

Status:

Closed (fixed)

» Active

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

Comment #19

catch

he/him

English

CreditAttribution: catch commented 19 October 2007 at 14:07

Status:

Active

» Needs work

seems reasonable and arguably a bug with the original patch, so setting back to CNW.

Comment #20

xamount CreditAttribution: xamount commented 20 November 2007 at 19:17

I second the backport to 5.x

Comment #21

chx CreditAttribution: chx commented 23 December 2007 at 10:40

Title:	Menu does support query and fragment in links	» Support fragment only links
Version:	6.x-dev	» 7.x-dev
Category:	bug	» feature
Status:	Needs work	» Active

Comment #22

sonicthoughts CreditAttribution: sonicthoughts commented 8 June 2008 at 06:55

please backport to 5.6/5.7. this has been very frustrating!
tx

Comment #23

kimothy777 CreditAttribution: kimothy777 commented 8 June 2008 at 06:57

I'm glad I'm not the only confused party here.

http://www.mychildcanbehave.com

Comment #24

pwolanin CreditAttribution: pwolanin commented 15 June 2008 at 15:04

Status:

Active

» Closed (works as designed)

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.