Skip to content

Loading…

Select options valid html #7025

Merged
merged 1 commit into from

2 participants

@rustygeldmacher

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.

Fixes issue #7017

Rusty Geldmacher 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
27c8deb
@rafaelfranca
Ruby on Rails member

Seems good. I think we can merge.

@carlosantoniodasilva anything to add?

@rafaelfranca
Ruby on Rails member

@rustygeldmacher could you prepare an another pull request to master branch?

@rustygeldmacher

Sure thing... looks like master has this problem but worse, where each option has the <div class="field_with_errors"> wrapping it. I'm working on a fix.

@rafaelfranca rafaelfranca was assigned
@rustygeldmacher

Just created pull request #7026 to fix this issue in master.

@rafaelfranca rafaelfranca merged commit 3b1de4a into rails:3-2-stable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 10, 2012
  1. Fixed bug creating invalid HTML in select options

    Rusty Geldmacher committed
    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
View
10 actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -134,7 +134,7 @@ module FormOptionsHelper
#
# ==== Gotcha
#
- # The HTML specification says when +multiple+ parameter passed to select and all options got deselected
+ # The HTML specification says when +multiple+ parameter passed to select and all options got deselected
# web browsers do not send any value to server. Unfortunately this introduces a gotcha:
# if an +User+ model has many +roles+ and have +role_ids+ accessor, and in the form that edits roles of the user
# 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)
end
- # Returns a string of option tags that have been compiled by iterating over the +collection+ and assigning
+ # Returns a string of option tags that have been compiled by iterating over the +collection+ and assigning
# the result of a call to the +value_method+ as the option value and the +text_method+ as the option text.
# Example:
# options_from_collection_for_select(@people, 'id', 'name')
@@ -616,11 +616,11 @@ def to_time_zone_select_tag(priority_zones, options, html_options)
private
def add_options(option_tags, options, value = nil)
if options[:include_blank]
- option_tags = content_tag('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags
+ option_tags = content_tag_string('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags
end
if value.blank? && options[:prompt]
prompt = options[:prompt].kind_of?(String) ? options[:prompt] : I18n.translate('helpers.select.prompt', :default => 'Please select')
- option_tags = content_tag('option', prompt, :value => '') + "\n" + option_tags
+ option_tags = content_tag_string('option', prompt, :value => '') + "\n" + option_tags
end
option_tags
end
@@ -630,7 +630,7 @@ def select_content_tag(option_tags, options, html_options)
add_default_name_and_id(html_options)
select = content_tag("select", add_options(option_tags, options, value(object)), html_options)
if html_options["multiple"]
- tag("input", :disabled => html_options["disabled"], :name => html_options["name"], :type => "hidden", :value => "") + select
+ tag("input", :disabled => html_options["disabled"], :name => html_options["name"], :type => "hidden", :value => "") + select
else
select
end
View
13 actionpack/test/template/active_model_helper_test.rb
@@ -41,6 +41,19 @@ def test_text_field_with_errors
)
end
+ def test_select_with_errors
+ assert_dom_equal(
+ %(<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>),
+ select("post", "author_name", [:a, :b])
+ )
+ end
+
+ def test_select_with_errors_and_blank_option
+ 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>)
+ assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], :include_blank => 'Choose one...'))
+ assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], :prompt => 'Choose one...'))
+ end
+
def test_date_select_with_errors
assert_dom_equal(
%(<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>),
Something went wrong with that request. Please try again.