Test fails when trying to patch routing.rb

Hi all

I'm trying to patch routing.rb to have the named route methods (home_url, login_url etc) html_escape the URL like url_for currently does. This is required when using XHTML 1.1 to have a well-formed document.

I've pastied the part of routing.rb that I've changed (NamedRouteCollection) here:

http://p.caboo.se/10380

This works fine in the browser, and I can use home_url(:foo => 'bar', :alpha => 'beta') and the URL will be properly escaped. But when i run rake in vendor/rails/actionpack one of the tests fails:

http://p.caboo.se/10381

It's an ActiveRecord test testing some session stuff, and I can't figure out why my changes are making it fail. I haven't set up any test database or anything like that, could that be the cause? When I run rake without my changes to routing.rb it doesn't fail, though, and I've traced the cause of the failure to line 127 in the first pastie. Does anyone have an idea why it's failing?

Also, is this patch likely to be accepted if I submit it? Should I change anything? I've refactored the code a bit to allow the helpers to use url_for, which has the :escape option, allowing the named helpers to accept this option too. Is there any tests I can write to check that my changes work properly?

I've also pastied the diff here in case anyone wants to test it for themselves:

http://p.caboo.se/10386

Thanks, Tore

Tore Darell wrote:

Hi all

I'm trying to patch routing.rb to have the named route methods (home_url, login_url etc) html_escape the URL like url_for currently does.

Keep in mind that this change should not touch routes very heavily. Remember that we do not want to escape the urls given back to controller url_for calls, so the change must be localized to the named route methods that are provided to the view.

It may be that installing named routes into ActionView::Base would fix this, as the url_for call will be issue to the view's url_for rather than the controller's.

Yes, this is what I was trying to do, but it turns out my changes were overly complicated and I was messing around too much.. What I've done now is create an additional array helper_names with just the route names in it (so url_helper_name and hash_access_name can be used in install), then define the *_url method directly on the helper module. The tests pass too now, after I stopped messing around.

Ticket + diff here: http://dev.rubyonrails.org/ticket/5932

Tore