Skip to content
This repository
Browse code

Fixed bug creating invalid HTML in select options

When a select tag is created for a field with errors, and that select
tag has :prompt or :include_blank options, then the inserted first
option will errantly have a <div class="field_with_errors"> wrapping
it.

See #7017
  • Loading branch information...
commit 27c8debdc6b242c845a279187205a2b057e18469 1 parent 45d78a3
rustygeldmacher authored July 10, 2012
10  actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -134,7 +134,7 @@ module FormOptionsHelper
134 134
       #
135 135
       # ==== Gotcha
136 136
       #
137  
-      # The HTML specification says when +multiple+ parameter passed to select and all options got deselected 
  137
+      # The HTML specification says when +multiple+ parameter passed to select and all options got deselected
138 138
       # web browsers do not send any value to server. Unfortunately this introduces a gotcha:
139 139
       # if an +User+ model has many +roles+ and have +role_ids+ accessor, and in the form that edits roles of the user
140 140
       # the user deselects all roles from +role_ids+ multiple select box, no +role_ids+ parameter is sent. So,
@@ -336,7 +336,7 @@ def options_for_select(container, selected = nil)
336 336
 
337 337
       end
338 338
 
339  
-      # Returns a string of option tags that have been compiled by iterating over the +collection+ and assigning 
  339
+      # Returns a string of option tags that have been compiled by iterating over the +collection+ and assigning
340 340
       # the result of a call to the +value_method+ as the option value and the +text_method+ as the option text.
341 341
       # Example:
342 342
       #   options_from_collection_for_select(@people, 'id', 'name')
@@ -616,11 +616,11 @@ def to_time_zone_select_tag(priority_zones, options, html_options)
616 616
       private
617 617
         def add_options(option_tags, options, value = nil)
618 618
           if options[:include_blank]
619  
-            option_tags = content_tag('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags
  619
+            option_tags = content_tag_string('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags
620 620
           end
621 621
           if value.blank? && options[:prompt]
622 622
             prompt = options[:prompt].kind_of?(String) ? options[:prompt] : I18n.translate('helpers.select.prompt', :default => 'Please select')
623  
-            option_tags = content_tag('option', prompt, :value => '') + "\n" + option_tags
  623
+            option_tags = content_tag_string('option', prompt, :value => '') + "\n" + option_tags
624 624
           end
625 625
           option_tags
626 626
         end
@@ -630,7 +630,7 @@ def select_content_tag(option_tags, options, html_options)
630 630
           add_default_name_and_id(html_options)
631 631
           select = content_tag("select", add_options(option_tags, options, value(object)), html_options)
632 632
           if html_options["multiple"]
633  
-            tag("input", :disabled => html_options["disabled"], :name => html_options["name"], :type => "hidden", :value => "") + select 
  633
+            tag("input", :disabled => html_options["disabled"], :name => html_options["name"], :type => "hidden", :value => "") + select
634 634
           else
635 635
             select
636 636
           end
13  actionpack/test/template/active_model_helper_test.rb
@@ -41,6 +41,19 @@ def test_text_field_with_errors
41 41
     )
42 42
   end
43 43
 
  44
+  def test_select_with_errors
  45
+    assert_dom_equal(
  46
+      %(<div class="field_with_errors"><select name="post[author_name]" id="post_author_name"><option value="a">a</option>\n<option value="b">b</option></select></div>),
  47
+      select("post", "author_name", [:a, :b])
  48
+    )
  49
+  end
  50
+
  51
+  def test_select_with_errors_and_blank_option
  52
+    expected_dom = %(<div class="field_with_errors"><select name="post[author_name]" id="post_author_name"><option value="">Choose one...</option>\n<option value="a">a</option>\n<option value="b">b</option></select></div>)
  53
+    assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], :include_blank => 'Choose one...'))
  54
+    assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], :prompt => 'Choose one...'))
  55
+  end
  56
+
44 57
   def test_date_select_with_errors
45 58
     assert_dom_equal(
46 59
       %(<div class="field_with_errors"><select id="post_updated_at_1i" name="post[updated_at(1i)]">\n<option selected="selected" value="2004">2004</option>\n<option value="2005">2005</option>\n</select>\n<input id="post_updated_at_2i" name="post[updated_at(2i)]" type="hidden" value="6" />\n<input id="post_updated_at_3i" name="post[updated_at(3i)]" type="hidden" value="15" />\n</div>),

0 notes on commit 27c8deb

Please sign in to comment.
Something went wrong with that request. Please try again.