Merge pull request #5102 from nashby/form-option-refactor

form option refactor

1 files changed, 13 insertions, 12 deletions

diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb
index bc03a1cf83..abb548c276 100644
--- a/actionpack/lib/action_view/helpers/form_options_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -330,9 +330,12 @@ module ActionView
         container.map do |element|
           html_attributes = option_html_attributes(element)
           text, value = option_text_and_value(element).map { |item| item.to_s }
-          selected_attribute = ' selected="selected"' if option_value_selected?(value, selected)
-          disabled_attribute = ' disabled="disabled"' if disabled && option_value_selected?(value, disabled)
-          %(<option value="#{ERB::Util.html_escape(value)}"#{selected_attribute}#{disabled_attribute}#{html_attributes}>#{ERB::Util.html_escape(text)}</option>)
+
+          html_attributes[:selected] = 'selected' if option_value_selected?(value, selected)
+          html_attributes[:disabled] = 'disabled' if disabled && option_value_selected?(value, disabled)
+          html_attributes[:value] = value
+
+          content_tag(:option, text, html_attributes)
         end.join("\n").html_safe
       end
 
@@ -472,16 +475,16 @@ module ActionView
       # <b>Note:</b> Only the <tt><optgroup></tt> and <tt><option></tt> tags are returned, so you still have to
       # wrap the output in an appropriate <tt><select></tt> tag.
       def grouped_options_for_select(grouped_options, selected_key = nil, prompt = nil)
-        body = ''
-        body << content_tag(:option, prompt, { :value => "" }, true) if prompt
+        body = "".html_safe
+        body.safe_concat content_tag(:option, prompt, :value => "") if prompt
 
         grouped_options = grouped_options.sort if grouped_options.is_a?(Hash)
 
-        grouped_options.each do |group|
-          body << content_tag(:optgroup, options_for_select(group[1], selected_key), :label => group[0])
+        grouped_options.each do |label, container|
+          body.safe_concat content_tag(:optgroup, options_for_select(container, selected_key), :label => label)
         end
 
-        body.html_safe
+        body
       end
 
       # Returns a string of option tags for pretty much any time zone in the
@@ -649,11 +652,9 @@ module ActionView
 
       private
         def option_html_attributes(element)
-          return "" unless Array === element
+          return {} unless Array === element
 
-          element.select { |e| Hash === e }.reduce({}, :merge).map do |k, v|
-            " #{k}=\"#{ERB::Util.html_escape(v.to_s)}\""
-          end.join
+          Hash[element.select { |e| Hash === e }.reduce({}, :merge).map { |k, v| [k, ERB::Util.html_escape(v.to_s)] }]
         end
 
         def option_text_and_value(option)