Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
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 rails#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.