Skip to content

Commit

Permalink
Consistently render button_to as <button>
Browse files Browse the repository at this point in the history
Prior to this commit, the
[ActionView::Helpers::UrlHelper#button_to][button_to] helper rendered
`<input type="submit">` elements when passed its contents as a String
argument, and rendered `<button type="submit">` elements when passed its
contents as a block.

This difference is subtle, and might lead to surprises.

Additionally, a `<form>` element's submitter can encode a `name`/`value`
pairing, which will be submitted as part of the request. When
`button_to` renders an `<input type="submit">` element, the "button"
content is rendered as a `[value]` attribute, which prevents any
meaningful data from being encoded.

Since it's a single `<button>` or `<input type="submit">` within a
`<form>`, missing out on that opportunity to encode information might
not be a show stopper, but ensuring that a `<button>` element is
rendered _without_ a default `[value]` attribute enables applications to
encode additional information that can be accessed JavaScript as
`element.value`, instead of a workaround like
`element.getAttribute("data-value")`.

Support rendering `input` elements with button_to
---

To support the original behavior of `button_to` rendering `<input
type="submit">` elements when invoked _without_ a block, expose the
`app.config.button_to_generates_button_tag` configuration flag.

By default, it's set to `true` and ensures that all `button_to` calls
render `<button>` elements. To revert to the original behavior, set it
to `false`.

[button_to]: https://api.rubyonrails.org/v6.0/classes/ActionView/Helpers/UrlHelper.html#method-i-button_to

Co-authored-by: Dusan Orlovic <duleorlovic@gmail.com>
  • Loading branch information
seanpdoyle and duleorlovic committed Dec 29, 2020
1 parent dbf6dbb commit 9af9458
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 32 deletions.
16 changes: 15 additions & 1 deletion actionview/CHANGELOG.md
@@ -1,7 +1,21 @@
* Change `ActionView::Helpers::UrlHelper#button_to` to *always* render a
`<button>` element, regardless of whether or not the content is passed as
the first argument or as a block

<%= button_to "Delete", post_path(@post), method: :delete %>
<%# => <form method="/posts/1"><input type="_method" value="delete"><button type="submit">Delete</button></form>

<%= button_to post_path(@post), method: :delete do %>
Delete
<% end %>
<%# => <form method="/posts/1"><input type="_method" value="delete"><button type="submit">Delete</button></form>

*Sean Doyle, Dusan Orlovic*

* Add `config.action_view.preload_links_header` to allow disabling of
the `Link` header being added by default when using `stylesheet_link_tag`
and `javascript_include_tag`.

*Andrew White*

* The `translate` helper now resolves `default` values when a `nil` key is
Expand Down
18 changes: 10 additions & 8 deletions actionview/lib/action_view/helpers/url_helper.rb
Expand Up @@ -29,6 +29,8 @@ def _url_for_modules
end
end

mattr_accessor :button_to_generates_button_tag, default: false

# Basic implementation of url_for to allow use helpers without routes existence
def url_for(options = nil) # :nodoc:
case options
Expand Down Expand Up @@ -250,12 +252,12 @@ def link_to(name = nil, options = nil, html_options = nil, &block)
# ==== Examples
# <%= button_to "New", action: "new" %>
# # => "<form method="post" action="/controller/new" class="button_to">
# # <input value="New" type="submit" />
# # <button type="submit">New</button>
# # </form>"
#
# <%= button_to "New", new_article_path %>
# # => "<form method="post" action="/articles/new" class="button_to">
# # <input value="New" type="submit" />
# # <button type="submit">New</button>
# # </form>"
#
# <%= button_to [:make_happy, @user] do %>
Expand All @@ -269,13 +271,13 @@ def link_to(name = nil, options = nil, html_options = nil, &block)
#
# <%= button_to "New", { action: "new" }, form_class: "new-thing" %>
# # => "<form method="post" action="/controller/new" class="new-thing">
# # <input value="New" type="submit" />
# # <button type="submit">New</button>
# # </form>"
#
#
# <%= button_to "Create", { action: "create" }, remote: true, form: { "data-type" => "json" } %>
# # => "<form method="post" action="/images/create" class="button_to" data-remote="true" data-type="json">
# # <input value="Create" type="submit" />
# # <button type="submit">Create</button>
# # <input name="authenticity_token" type="hidden" value="10f2163b45388899ad4d5ae948988266befcb6c3d1b2451cf657a0c293d605a6"/>
# # </form>"
#
Expand All @@ -284,7 +286,7 @@ def link_to(name = nil, options = nil, html_options = nil, &block)
# method: :delete, data: { confirm: "Are you sure?" } %>
# # => "<form method="post" action="/images/delete/1" class="button_to">
# # <input type="hidden" name="_method" value="delete" />
# # <input data-confirm='Are you sure?' value="Delete Image" type="submit" />
# # <button data-confirm='Are you sure?' type="submit">Delete Image</button>
# # <input name="authenticity_token" type="hidden" value="10f2163b45388899ad4d5ae948988266befcb6c3d1b2451cf657a0c293d605a6"/>
# # </form>"
#
Expand All @@ -293,7 +295,7 @@ def link_to(name = nil, options = nil, html_options = nil, &block)
# method: :delete, remote: true, data: { confirm: 'Are you sure?', disable_with: 'loading...' }) %>
# # => "<form class='button_to' method='post' action='http://www.example.com' data-remote='true'>
# # <input name='_method' value='delete' type='hidden' />
# # <input value='Destroy' type='submit' data-disable-with='loading...' data-confirm='Are you sure?' />
# # <button type='submit' data-disable-with='loading...' data-confirm='Are you sure?'>Destroy</button>
# # <input name="authenticity_token" type="hidden" value="10f2163b45388899ad4d5ae948988266befcb6c3d1b2451cf657a0c293d605a6"/>
# # </form>"
# #
Expand Down Expand Up @@ -327,8 +329,8 @@ def button_to(name = nil, options = nil, html_options = nil, &block)
html_options = convert_options_to_data_attributes(options, html_options)
html_options["type"] = "submit"

button = if block_given?
content_tag("button", html_options, &block)
button = if block_given? || button_to_generates_button_tag
content_tag("button", name || url, html_options, &block)
else
html_options["value"] = name || url
tag("input", html_options)
Expand Down
7 changes: 7 additions & 0 deletions actionview/lib/action_view/railtie.rb
Expand Up @@ -81,6 +81,13 @@ class Railtie < Rails::Engine # :nodoc:
PartialRenderer.collection_cache = app.config.action_controller.cache_store
end

initializer "action_view.button_to_generates_button_tag" do |app|
ActiveSupport.on_load(:action_view) do
ActionView::Helpers::UrlHelper.button_to_generates_button_tag =
app.config.action_view.delete(:button_to_generates_button_tag)
end
end

config.after_initialize do |app|
enable_caching = if app.config.action_view.cache_template_loading.nil?
app.config.cache_classes
Expand Down
64 changes: 41 additions & 23 deletions actionview/test/template/url_helper_test.rb
Expand Up @@ -50,6 +50,8 @@ class UrlHelperTest < ActiveSupport::TestCase
include RenderERBUtils

setup :_prepare_context
setup { ActionView::Helpers::UrlHelper.button_to_generates_button_tag = @button_to_generates_button_tag = true }
teardown { ActionView::Helpers::UrlHelper.button_to_generates_button_tag = @button_to_generates_button_tag }

def hash_for(options = {})
{ controller: "foo", action: "bar" }.merge!(options)
Expand Down Expand Up @@ -140,20 +142,20 @@ def test_to_form_params_with_namespace
def test_button_to_without_protect_against_forgery_method
self.class.undef_method(:protect_against_forgery?)
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to"><button type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com")
)
ensure
self.class.define_method(:protect_against_forgery?) { request_forgery }
end

def test_button_to_with_straight_url
assert_dom_equal %{<form method="post" action="http://www.example.com" class="button_to"><input type="submit" value="Hello" /></form>}, button_to("Hello", "http://www.example.com")
assert_dom_equal %{<form method="post" action="http://www.example.com" class="button_to"><button type="submit">Hello</button></form>}, button_to("Hello", "http://www.example.com")
end

def test_button_to_with_path
assert_dom_equal(
%{<form method="post" action="/article/Hello" class="button_to"><input type="submit" value="Hello" /></form>},
%{<form method="post" action="/article/Hello" class="button_to"><button type="submit">Hello</button></form>},
button_to("Hello", article_path("Hello"))
)
end
Expand All @@ -162,96 +164,100 @@ def test_button_to_with_straight_url_and_request_forgery
self.request_forgery = true

assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input type="submit" value="Hello" /><input name="form_token" type="hidden" value="secret" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to"><button type="submit">Hello</button><input name="form_token" type="hidden" value="secret" /></form>},
button_to("Hello", "http://www.example.com")
)
ensure
self.request_forgery = false
end

def test_button_to_with_form_class
assert_dom_equal %{<form method="post" action="http://www.example.com" class="custom-class"><input type="submit" value="Hello" /></form>}, button_to("Hello", "http://www.example.com", form_class: "custom-class")
assert_dom_equal %{<form method="post" action="http://www.example.com" class="custom-class"><button type="submit">Hello</button></form>}, button_to("Hello", "http://www.example.com", form_class: "custom-class")
end

def test_button_to_with_form_class_escapes
assert_dom_equal %{<form method="post" action="http://www.example.com" class="&lt;script&gt;evil_js&lt;/script&gt;"><input type="submit" value="Hello" /></form>}, button_to("Hello", "http://www.example.com", form_class: "<script>evil_js</script>")
assert_dom_equal %{<form method="post" action="http://www.example.com" class="&lt;script&gt;evil_js&lt;/script&gt;"><button type="submit">Hello</button></form>}, button_to("Hello", "http://www.example.com", form_class: "<script>evil_js</script>")
end

def test_button_to_with_query
assert_dom_equal %{<form method="post" action="http://www.example.com/q1=v1&amp;q2=v2" class="button_to"><input type="submit" value="Hello" /></form>}, button_to("Hello", "http://www.example.com/q1=v1&q2=v2")
assert_dom_equal %{<form method="post" action="http://www.example.com/q1=v1&amp;q2=v2" class="button_to"><button type="submit">Hello</button></form>}, button_to("Hello", "http://www.example.com/q1=v1&q2=v2")
end

def test_button_to_with_value
assert_dom_equal %{<form method="post" action="http://www.example.com" class="button_to"><button type="submit" name="key" value="value">Hello</button></form>}, button_to("Hello", "http://www.example.com", name: "key", value: "value")
end

def test_button_to_with_html_safe_URL
assert_dom_equal %{<form method="post" action="http://www.example.com/q1=v1&amp;q2=v2" class="button_to"><input type="submit" value="Hello" /></form>}, button_to("Hello", raw("http://www.example.com/q1=v1&amp;q2=v2"))
assert_dom_equal %{<form method="post" action="http://www.example.com/q1=v1&amp;q2=v2" class="button_to"><button type="submit">Hello</button></form>}, button_to("Hello", raw("http://www.example.com/q1=v1&amp;q2=v2"))
end

def test_button_to_with_query_and_no_name
assert_dom_equal %{<form method="post" action="http://www.example.com?q1=v1&amp;q2=v2" class="button_to"><input type="submit" value="http://www.example.com?q1=v1&amp;q2=v2" /></form>}, button_to(nil, "http://www.example.com?q1=v1&q2=v2")
assert_dom_equal %{<form method="post" action="http://www.example.com?q1=v1&amp;q2=v2" class="button_to"><button type="submit">http://www.example.com?q1=v1&amp;q2=v2</button></form>}, button_to(nil, "http://www.example.com?q1=v1&q2=v2")
end

def test_button_to_with_javascript_confirm
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input data-confirm="Are you sure?" type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to"><button data-confirm="Are you sure?" type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", data: { confirm: "Are you sure?" })
)
end

def test_button_to_with_javascript_disable_with
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input data-disable-with="Greeting..." type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to"><button data-disable-with="Greeting..." type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", data: { disable_with: "Greeting..." })
)
end

def test_button_to_with_remote_and_form_options
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="custom-class" data-remote="true" data-type="json"><input type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="custom-class" data-remote="true" data-type="json"><button type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", remote: true, form: { class: "custom-class", "data-type" => "json" })
)
end

def test_button_to_with_remote_and_javascript_confirm
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to" data-remote="true"><input data-confirm="Are you sure?" type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to" data-remote="true"><button data-confirm="Are you sure?" type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", remote: true, data: { confirm: "Are you sure?" })
)
end

def test_button_to_with_remote_and_javascript_disable_with
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to" data-remote="true"><input data-disable-with="Greeting..." type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to" data-remote="true"><button data-disable-with="Greeting..." type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", remote: true, data: { disable_with: "Greeting..." })
)
end

def test_button_to_with_remote_false
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to"><button type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", remote: false)
)
end

def test_button_to_enabled_disabled
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to"><button type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", disabled: false)
)
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input disabled="disabled" type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to"><button disabled="disabled" type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", disabled: true)
)
end

def test_button_to_with_method_delete
assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input type="hidden" name="_method" value="delete" /><input type="submit" value="Hello" /></form>},
%{<form method="post" action="http://www.example.com" class="button_to"><input type="hidden" name="_method" value="delete" /><button type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", method: :delete)
)
end

def test_button_to_with_method_get
assert_dom_equal(
%{<form method="get" action="http://www.example.com" class="button_to"><input type="submit" value="Hello" /></form>},
%{<form method="get" action="http://www.example.com" class="button_to"><button type="submit">Hello</button></form>},
button_to("Hello", "http://www.example.com", method: :get)
)
end
Expand All @@ -265,11 +271,23 @@ def test_button_to_with_block

def test_button_to_with_params
assert_dom_equal(
%{<form action="http://www.example.com" class="button_to" method="post"><input type="submit" value="Hello" /><input type="hidden" name="baz" value="quux" /><input type="hidden" name="foo" value="bar" /></form>},
%{<form action="http://www.example.com" class="button_to" method="post"><button type="submit">Hello</button><input type="hidden" name="baz" value="quux" /><input type="hidden" name="foo" value="bar" /></form>},
button_to("Hello", "http://www.example.com", params: { foo: :bar, baz: "quux" })
)
end

def test_button_to_generates_input_when_button_to_generates_button_tag_false
old_value = ActionView::Helpers::UrlHelper.button_to_generates_button_tag
ActionView::Helpers::UrlHelper.button_to_generates_button_tag = false

assert_dom_equal(
%{<form method="post" action="http://www.example.com" class="button_to"><input type="submit" value="Save"/></form>},
button_to("Save", "http://www.example.com")
)
ensure
ActionView::Helpers::UrlHelper.button_to_generates_button_tag = old_value
end

class FakeParams
def initialize(permitted = true)
@permitted = permitted
Expand All @@ -290,7 +308,7 @@ def to_h

def test_button_to_with_permitted_strong_params
assert_dom_equal(
%{<form action="http://www.example.com" class="button_to" method="post"><input type="submit" value="Hello" /><input type="hidden" name="baz" value="quux" /><input type="hidden" name="foo" value="bar" /></form>},
%{<form action="http://www.example.com" class="button_to" method="post"><button type="submit">Hello</button><input type="hidden" name="baz" value="quux" /><input type="hidden" name="foo" value="bar" /></form>},
button_to("Hello", "http://www.example.com", params: FakeParams.new)
)
end
Expand All @@ -303,14 +321,14 @@ def test_button_to_with_unpermitted_strong_params

def test_button_to_with_nested_hash_params
assert_dom_equal(
%{<form action="http://www.example.com" class="button_to" method="post"><input type="submit" value="Hello" /><input type="hidden" name="foo[bar]" value="baz" /></form>},
%{<form action="http://www.example.com" class="button_to" method="post"><button type="submit">Hello</button><input type="hidden" name="foo[bar]" value="baz" /></form>},
button_to("Hello", "http://www.example.com", params: { foo: { bar: "baz" } })
)
end

def test_button_to_with_nested_array_params
assert_dom_equal(
%{<form action="http://www.example.com" class="button_to" method="post"><input type="submit" value="Hello" /><input type="hidden" name="foo[]" value="bar" /></form>},
%{<form action="http://www.example.com" class="button_to" method="post"><button type="submit">Hello</button><input type="hidden" name="foo[]" value="bar" /></form>},
button_to("Hello", "http://www.example.com", params: { foo: ["bar"] })
)
end
Expand Down
4 changes: 4 additions & 0 deletions railties/lib/rails/application/configuration.rb
Expand Up @@ -203,6 +203,10 @@ def load_defaults(target_version)
ActiveSupport.utc_to_local_returns_utc_offset_times = true
when "6.2"
load_defaults "6.1"

if respond_to?(:action_view)
action_view.button_to_generates_button_tag = true
end
else
raise "Unknown version #{target_version.to_s.inspect}"
end
Expand Down
Expand Up @@ -5,3 +5,6 @@
# Once upgraded flip defaults one by one to migrate to the new default.
#
# Read the Guide for Upgrading Ruby on Rails for more info on each option.

# button_to view helpers consistently render <button> elements.
# Rails.application.config.action_view.button_to_generates_button_tag = false

0 comments on commit 9af9458

Please sign in to comment.