#4147 link_to generates incorrect hrefs - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4147 ✓resolved

link_to generates incorrect hrefs

Reported by Sam Ruby | March 10th, 2010 @ 04:16 PM | in 3.0.2

link_to "Goodbye!", say_goodbye_path produces

<a href="http://localhost:3000/say/hello/say/goodbye">Goodbye!</a>

Broken by commit f38e2e03351da463f84f6850fa10718ece98ff26.

testcase, output, git bisect

Comments and changes to this ticket

Jeremy Kemper March 11th, 2010 @ 12:09 AM
- State changed from “new” to “open”
- Assigned user set to “Carl Lerche”
You flagged this item as spam.
Denis Odorcic March 16th, 2010 @ 05:50 AM
- Tag set to “default_url_options, patch, script_name, url_for”
So I looked into this issue and it seems that script_name was introduced to url_for's url_options. It then gets prepended to the path that url_for generates, however it should only be prepending it when the user passes it in as far as I can tell. In this example, say/hello is the script name that the controller picks up and ends up prepending it to say/goodbye.

I've attached a patch, a couple of tests for the above cases I described, and also a 3rd fix on an existing test that failed after I made this fix. I took a close look at the test and as far as I can tell there's no reason that its url should have 'bar' in it, but if someone could double check that would be great.
- test-fix-url-generation-from-url-for.diff 2 KB
You flagged this item as spam.
Denis Odorcic March 16th, 2010 @ 06:00 AM
Deleted the fix of an existing test as it was reverted a few minutes ago by b9c48f519
You flagged this item as spam.
Denis Odorcic March 16th, 2010 @ 06:54 AM
- Tag changed from “default_url_options, patch, script_name, url_for” to “default_url_options, script_name, url_for”
Spoke too soon, not all tests were passing, at least you guys are pointed in the right direction :) I think the failing tests I added should be correct though, so I'll leave that up here.
José Valim March 17th, 2010 @ 08:19 AM
Sam, this seems to be fixed on master. Could you please check?
You flagged this item as spam.
Denis Odorcic March 17th, 2010 @ 12:55 PM
Looks like it has been fixed in master now, yes.
José Valim March 17th, 2010 @ 09:31 PM
- State changed from “open” to “resolved”
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
- Importance changed from “” to “High”
[bulk edit]