Skip to content
This repository
Browse code

UrlRewriter#rewrite_url should call #to_param on the value given in :…

…anchor option, just as #url_for does

[#2746 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
commit 05b529ca57f75ce64540b9d34597e0c3bfe1fca7 1 parent cc9af20
Jeffrey Hardy authored jeremy committed
2  actionpack/lib/action_controller/routing/generation/url_rewriter.rb
@@ -172,7 +172,7 @@ def rewrite_url(options)
172 172
         path = rewrite_path(options)
173 173
         rewritten_url << ActionController::Base.relative_url_root.to_s unless options[:skip_relative_url_root]
174 174
         rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
175  
-        rewritten_url << "##{options[:anchor]}" if options[:anchor]
  175
+        rewritten_url << "##{CGI.escape(options[:anchor].to_param.to_s)}" if options[:anchor]
176 176
 
177 177
         rewritten_url
178 178
       end
26  actionpack/test/controller/url_rewriter_test.rb
@@ -46,6 +46,20 @@ def test_anchor
46 46
     )
47 47
   end
48 48
 
  49
+  def test_anchor_should_call_to_param
  50
+    assert_equal(
  51
+      'http://test.host/c/a/i#anchor',
  52
+      @rewriter.rewrite(:controller => 'c', :action => 'a', :id => 'i', :anchor => Struct.new(:to_param).new('anchor'))
  53
+    )
  54
+  end
  55
+
  56
+  def test_anchor_should_be_cgi_escaped
  57
+    assert_equal(
  58
+      'http://test.host/c/a/i#anc%2Fhor',
  59
+      @rewriter.rewrite(:controller => 'c', :action => 'a', :id => 'i', :anchor => Struct.new(:to_param).new('anc/hor'))
  60
+    )
  61
+  end
  62
+
49 63
   def test_overwrite_params
50 64
     @params[:controller] = 'hi'
51 65
     @params[:action] = 'bye'
@@ -110,6 +124,18 @@ def test_anchor
110 124
     )
111 125
   end
112 126
 
  127
+  def test_anchor_should_call_to_param
  128
+    assert_equal('/c/a#anchor',
  129
+      W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => Struct.new(:to_param).new('anchor'))
  130
+    )
  131
+  end
  132
+
  133
+  def test_anchor_should_be_cgi_escaped
  134
+    assert_equal('/c/a#anc%2Fhor',
  135
+      W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => Struct.new(:to_param).new('anc/hor'))
  136
+    )
  137
+  end
  138
+
113 139
   def test_default_host
114 140
     add_host!
115 141
     assert_equal('http://www.basecamphq.com/c/a/i',

1 note on commit 05b529c

Adam Williams

The ActionView::Helpers::UrlHelper#url_for method allows for an :escape => false option. This allowed us to have unescaped :anchor options. That is lost with this change. Was that intentional?

Please sign in to comment.
Something went wrong with that request. Please try again.