Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Accept :remote as symbol in link_to options #7864

Merged
merged 1 commit into from

2 participants

@teleological

The documentation for UrlHelper#link_to describes a :remote option, which can be set in either the html_options or (url_)options hashes. But the option is checked as a stringified key ('remote'), and only the html_options hash is stringified before this check takes place, so that :remote is ignored if passed in the other hash as a symbol.

This problem exists on 3.2-stable, but an additional issue exists on master: Master passes the options hash to UrlHelper#url_for before consuming and deleting the :remote option. As a result, both the string and symbol forms are used for url-generation before they are used to set the data-remote attribute. This patch also addresses this issue.

I pushed a 3-2 backport to link_to_remote_3_2 in my repository, in case that's useful. Thanks!

@teleological teleological Accept :remote as symbol in link_to options
Accept either :remote or 'remote' in both the html_options and
(url_)options hash arguments to link_to.
6caae02
@rafaelfranca rafaelfranca commented on the diff
actionpack/test/template/url_helper_test.rb
@@ -298,6 +298,20 @@ def test_link_to_with_remote_false
)
end
+ def test_link_to_with_symbolic_remote_in_non_html_options
+ assert_dom_equal(
+ "<a href=\"/\" data-remote=\"true\">Hello</a>",
+ link_to("Hello", hash_for(:remote => true), {})
+ )
+ end
+
+ def test_link_to_with_string_remote_in_non_html_options
+ assert_dom_equal(
+ "<a href=\"/\" data-remote=\"true\">Hello</a>",
+ link_to("Hello", hash_for('remote' => true), {})
@rafaelfranca Owner

This call is not right. the call should be

link_to("Hello", hash_for, :remote => true)
@rafaelfranca Owner

You are right this is supported too, but I don't think this test is asserting the behavior since it only doesn't stringify the keys when the html_options is not there. But in this case it is there.

The test with 'remote' (as a String) also fails without the changes to UrlHelper, but only on the master branch. The test with :remote as a symbol fails on both master and 3-2-stable. Both tests pass on both branches with the included changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rafaelfranca rafaelfranca merged commit 410d376 into rails:master
@rafaelfranca
Owner

Right. Thanks

@rafaelfranca
Owner

Could you open a pull request with the 3-2-stable backport?

@teleological

Done, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 6, 2012
  1. @teleological

    Accept :remote as symbol in link_to options

    teleological authored
    Accept either :remote or 'remote' in both the html_options and
    (url_)options hash arguments to link_to.
This page is out of date. Refresh to see the latest.
View
2  actionpack/CHANGELOG.md
@@ -1,5 +1,7 @@
## Rails 4.0.0 (unreleased) ##
+* Accept :remote as symbolic option for `link_to` helper. *Riley Lynch*
+
* Warn when the `:locals` option is passed to `assert_template` outside of a view test case
Fix #3415
View
7 actionpack/lib/action_view/helpers/url_helper.rb
@@ -175,9 +175,10 @@ def _back_url # :nodoc:
def link_to(name = nil, options = nil, html_options = nil, &block)
html_options, options = options, name if block_given?
options ||= {}
- url = url_for(options)
html_options = convert_options_to_data_attributes(options, html_options)
+
+ url = url_for(options)
html_options['href'] ||= url
content_tag(:a, name || url, html_options, &block)
@@ -598,7 +599,9 @@ def convert_options_to_data_attributes(options, html_options)
end
def link_to_remote_options?(options)
- options.is_a?(Hash) && options.delete('remote')
+ if options.is_a?(Hash)
+ options.delete('remote') || options.delete(:remote)
+ end
end
def add_method_to_attributes!(html_options, method)
View
14 actionpack/test/template/url_helper_test.rb
@@ -298,6 +298,20 @@ def test_link_to_with_remote_false
)
end
+ def test_link_to_with_symbolic_remote_in_non_html_options
+ assert_dom_equal(
+ "<a href=\"/\" data-remote=\"true\">Hello</a>",
+ link_to("Hello", hash_for(:remote => true), {})
+ )
+ end
+
+ def test_link_to_with_string_remote_in_non_html_options
+ assert_dom_equal(
+ "<a href=\"/\" data-remote=\"true\">Hello</a>",
+ link_to("Hello", hash_for('remote' => true), {})
@rafaelfranca Owner

This call is not right. the call should be

link_to("Hello", hash_for, :remote => true)
@rafaelfranca Owner

You are right this is supported too, but I don't think this test is asserting the behavior since it only doesn't stringify the keys when the html_options is not there. But in this case it is there.

The test with 'remote' (as a String) also fails without the changes to UrlHelper, but only on the master branch. The test with :remote as a symbol fails on both master and 3-2-stable. Both tests pass on both branches with the included changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ )
+ end
+
def test_link_tag_using_post_javascript
assert_dom_equal(
"<a href='http://www.example.com' data-method=\"post\" rel=\"nofollow\">Hello</a>",
Something went wrong with that request. Please try again.