Merge pull request #20124 from greysteil/fix-select-helper

Raise an ArgumentError when `include_blank` is false for a required select field

actionview/CHANGELOG.md

+*   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.
 

actionview/lib/action_view/helpers/tags/base.rb

@@ -120,7 +120,12 @@ def sanitized_value(value)
         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 @@ def select_content_tag(option_tags, options, html_options)
           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)

actionview/test/template/form_options_helper_test.rb

@@ -645,6 +645,13 @@ def test_select_with_blank
     )
   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>"