Fix double url escaping of remote_function. Add :escape => false option to ActionView's url_for.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4014 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 23 insertions, 3 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 46a1861f67..7fecc9451a 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fix double url escaping of remote_function. Add :escape => false option to ActionView's url_for. [Nicholas Seckar]
+
 * Add :script option to in_place_editor to support evalScripts (closes #4194) [codyfauser@gmail.com]
 
 * Fix mixed case enumerable methods in the JavaScript Collection Proxy (closes #4314) [codyfauser@gmail.com]
diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb
index de964edae8..40bb18c624 100644
--- a/actionpack/lib/action_view/helpers/prototype_helper.rb
+++ b/actionpack/lib/action_view/helpers/prototype_helper.rb
@@ -301,7 +301,9 @@ module ActionView
           "new Ajax.Request(" :
           "new Ajax.Updater(#{update}, "
 
-        function << "'#{url_for(options[:url])}'"
+        url_options = options[:url]
+        url_options = url_options.merge(:escape => false) if url_options.is_a? Hash
+        function << "'#{url_for(url_options)}'"
         function << ", #{javascript_options})"
 
         function = "#{options[:before]}; #{function}" if options[:before]
diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index ae5f0c230d..5cbf728961 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -13,9 +13,19 @@ module ActionView
       # as url_for. For a list, see the documentation for ActionController::Base#url_for.
       # Note that it'll set :only_path => true so you'll get /controller/action instead of the 
       # http://example.com/controller/action part (makes it harder to parse httpd log files)
+      # 
+      # When called from a view, url_for returns an HTML escaped url. If you need an unescaped
+      # url, pass :escape => false to url_for.
+      # 
       def url_for(options = {}, *parameters_for_method_reference)
-        options = { :only_path => true }.update(options.symbolize_keys) if options.kind_of? Hash
-        html_escape(@controller.send(:url_for, options, *parameters_for_method_reference))
+        if options.kind_of? Hash
+          options = { :only_path => true }.update(options.symbolize_keys)
+          escape = options.key?(:escape) ? options.delete(:escape) : true
+        else
+          escape = true
+        end
+        url = @controller.send(:url_for, options, *parameters_for_method_reference)
+        escape ? html_escape(url) : url
       end
 
       # Creates a link tag of the given +name+ using an URL created by the set of +options+. See the valid options in
diff --git a/actionpack/test/template/prototype_helper_test.rb b/actionpack/test/template/prototype_helper_test.rb
index 71ba44d227..ceeb418e34 100644
--- a/actionpack/test/template/prototype_helper_test.rb
+++ b/actionpack/test/template/prototype_helper_test.rb
@@ -16,6 +16,8 @@ module BaseTest
       def url_for(options, *parameters_for_method_reference)
         url =  "http://www.example.com/"
         url << options[:action].to_s if options and options[:action]
+        url << "?a=#{options[:a]}" if options && options[:a]
+        url << "&b=#{options[:b]}" if options && options[:a] && options[:b]
         url
       end
     end.new
@@ -40,6 +42,8 @@ class PrototypeHelperTest < Test::Unit::TestCase
       link_to_remote("Remote outpost", :success => "alert(request.reponseText)", :url => { :action => "whatnot"  })
     assert_dom_equal %(<a href=\"#\" onclick=\"new Ajax.Request('http://www.example.com/whatnot', {asynchronous:true, evalScripts:true, onFailure:function(request){alert(request.reponseText)}}); return false;\">Remote outpost</a>),
       link_to_remote("Remote outpost", :failure => "alert(request.reponseText)", :url => { :action => "whatnot"  })
+    assert_dom_equal %(<a href=\"#\" onclick=\"new Ajax.Request('http://www.example.com/whatnot?a=10&amp;b=20', {asynchronous:true, evalScripts:true, onFailure:function(request){alert(request.reponseText)}}); return false;\">Remote outpost</a>),
+      link_to_remote("Remote outpost", :failure => "alert(request.reponseText)", :url => { :action => "whatnot", :a => '10', :b => '20' })
   end
   
   def test_periodically_call_remote
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index 2c7a526315..231e34751e 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -25,6 +25,8 @@ class UrlHelperTest < Test::Unit::TestCase
   def test_url_for_escapes_urls
     @controller.url = "http://www.example.com?a=b&c=d"
     assert_equal "http://www.example.com?a=b&amp;c=d", url_for(:a => 'b', :c => 'd')
+    assert_equal "http://www.example.com?a=b&amp;c=d", url_for(:a => 'b', :c => 'd', :escape => true)
+    assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd', :escape => false)
   end
   
   # todo: missing test cases