diff options
| author | Grey Baker <greysteil@gmail.com> | 2015-05-12 15:10:20 +0100 |
|---|---|---|
| committer | Grey Baker <greysteil@gmail.com> | 2015-06-08 16:41:44 +0100 |
| commit | 077540738460d44c1463b3e52d2acb1fed9bc766 (patch) | |
| tree | 7372acefce4ae672f204c07f6282bd2916724aa0 /actionview | |
| parent | aeac00feb41ec99ef39b79cae31546ca875188d9 (diff) | |
| download | rails-077540738460d44c1463b3e52d2acb1fed9bc766.tar.gz rails-077540738460d44c1463b3e52d2acb1fed9bc766.tar.bz2 rails-077540738460d44c1463b3e52d2acb1fed9bc766.zip | |
Raise an ArgumentError when `include_blank` is false for a required field in
`Tags::Base#select_content_tag`.
Previously, passing a falsey value to `include_blank` would be ignored if the
field was required, and a blank line would still be inserted. The following will
now raise instead of quietly failing:
`select("post", "category", %w(a required field), { include_blank: false }, required: 'required')`
Diffstat (limited to 'actionview')
| -rw-r--r-- | actionview/CHANGELOG.md | 5 | ||||
| -rw-r--r-- | actionview/lib/action_view/helpers/tags/base.rb | 12 | ||||
| -rw-r--r-- | actionview/test/template/form_options_helper_test.rb | 7 |
3 files changed, 21 insertions, 3 deletions
diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 201e118971..7b6008d5ed 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,8 @@ +* Raise an ArgumentError when a false value for `include_blank` is passed to a + required select field (to comply with the HTML5 spec). + + *Grey Baker* + * Do not put partial name to `local_assigns` when rendering without an object or a collection. diff --git a/actionview/lib/action_view/helpers/tags/base.rb b/actionview/lib/action_view/helpers/tags/base.rb index acc6443a96..d57f26ba4f 100644 --- a/actionview/lib/action_view/helpers/tags/base.rb +++ b/actionview/lib/action_view/helpers/tags/base.rb @@ -120,7 +120,12 @@ module ActionView def select_content_tag(option_tags, options, html_options) html_options = html_options.stringify_keys add_default_name_and_id(html_options) - options[:include_blank] ||= true unless options[:prompt] || select_not_required?(html_options) + + if placeholder_required?(html_options) + raise ArgumentError, "include_blank cannot be false for a required field." if options[:include_blank] == false + options[:include_blank] ||= true unless options[:prompt] + end + value = options.fetch(:selected) { value(object) } select = content_tag("select", add_options(option_tags, options, value), html_options) @@ -131,8 +136,9 @@ module ActionView end end - def select_not_required?(html_options) - !html_options["required"] || html_options["multiple"] || html_options["size"].to_i > 1 + def placeholder_required?(html_options) + # See https://html.spec.whatwg.org/multipage/forms.html#attr-select-required + html_options["required"] && !html_options["multiple"] && html_options.fetch("size", 1).to_i == 1 end def add_options(option_tags, options, value = nil) diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index d25fa3706f..d7daba8bf3 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -645,6 +645,13 @@ class FormOptionsHelperTest < ActionView::TestCase ) end + def test_select_with_include_blank_false_and_required + @post = Post.new + @post.category = "<mus>" + e = assert_raises(ArgumentError) { select("post", "category", %w( abe <mus> hest), { include_blank: false }, required: 'required') } + assert_match(/include_blank cannot be false for a required field./, e.message) + end + def test_select_with_blank_as_string @post = Post.new @post.category = "<mus>" |