Convert boolean form options form the tag_helper. Recloses #809.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2523 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Convert boolean form options form the tag_helper. #809. [Michael Schuerig <michael@schuerig.de>]
+
 * Fixed that an instance variable with the same name as a partial should be implicitly passed as the partial :object #2269 [court3nay]
 
 * Update Prototype to V1.4.0_pre11, script.aculo.us to [2502] [Thomas Fuchs]

actionpack/lib/action_view/helpers/form_tag_helper.rb

@@ -47,7 +47,7 @@ module ActionView
       # Options:
       # * <tt>:multiple</tt> - If set to true the selection will allow multiple choices.
       def select_tag(name, option_tags = nil, options = {})
-        content_tag("select", option_tags, { "name" => name, "id" => name }.update(convert_options(options)))
+        content_tag("select", option_tags, { "name" => name, "id" => name }.update(options))
       end
 
       # Creates a standard text field.
@@ -59,7 +59,7 @@ module ActionView
       # 
       # A hash of standard HTML options for the tag.
       def text_field_tag(name, value = nil, options = {})
-        tag("input", { "type" => "text", "name" => name, "id" => name, "value" => value }.update(convert_options(options)))
+        tag("input", { "type" => "text", "name" => name, "id" => name, "value" => value }.update(options))
       end
 
       # Creates a hidden field.
@@ -80,14 +80,14 @@ module ActionView
       # The specified URL will then be passed a File object containing the selected file, or if the field 
       # was left blank, a StringIO object.
       def file_field_tag(name, options = {})
-        text_field_tag(name, nil, convert_options(options).update("type" => "file"))
+        text_field_tag(name, nil, options.update("type" => "file"))
       end
 
       # Creates a password field.
       #
       # Takes the same options as text_field_tag
       def password_field_tag(name = "password", value = nil, options = {})
-        text_field_tag(name, value, convert_options(options).update("type" => "password"))
+        text_field_tag(name, value, options.update("type" => "password"))
       end
 
       # Creates a text input area.
@@ -103,49 +103,34 @@ module ActionView
           options.delete("size")
         end
 
-        content_tag("textarea", content, { "name" => name, "id" => name }.update(convert_options(options)))
+        content_tag("textarea", content, { "name" => name, "id" => name }.update(options))
       end
 
       # Creates a check box.
       def check_box_tag(name, value = "1", checked = false, options = {})
-        html_options = { "type" => "checkbox", "name" => name, "id" => name, "value" => value }.update(convert_options(options))
+        html_options = { "type" => "checkbox", "name" => name, "id" => name, "value" => value }.update(options)
         html_options["checked"] = "checked" if checked
         tag("input", html_options)
       end
 
       # Creates a radio button.
       def radio_button_tag(name, value, checked = false, options = {})
-        html_options = { "type" => "radio", "name" => name, "id" => name, "value" => value }.update(convert_options(options))
+        html_options = { "type" => "radio", "name" => name, "id" => name, "value" => value }.update(options)
         html_options["checked"] = "checked" if checked
         tag("input", html_options)
       end
 
       # Creates a submit button with the text <tt>value</tt> as the caption.
       def submit_tag(value = "Save changes", options = {})
-        tag("input", { "type" => "submit", "name" => "commit", "value" => value }.update(convert_options(options)))
+        tag("input", { "type" => "submit", "name" => "commit", "value" => value }.update(options))
       end
       
       # Displays an image which when clicked will submit the form.
       #
       # <tt>source</tt> is passed to AssetTagHelper#image_path
       def image_submit_tag(source, options = {})
-        tag("input", { "type" => "image", "src" => image_path(source) }.update(convert_options(options)))
+        tag("input", { "type" => "image", "src" => image_path(source) }.update(options))
       end
-      
-      private
-        def convert_options(options)
-          options = options.stringify_keys
-          %w( disabled readonly multiple ).each { |a| boolean_attribute(options, a) }
-          options
-        end
-        
-        def boolean_attribute(options, attribute)
-          if options[attribute]
-            options[attribute] = attribute
-          else
-            options.delete attribute
-          end
-        end
     end
   end
 end

actionpack/lib/action_view/helpers/tag_helper.rb

@@ -24,11 +24,17 @@ module ActionView
 
       private
         def tag_options(options)
-          if options
-            options.inject("") do |html_str, (key, value)|
-              value.nil? ? html_str : html_str << %( #{key}="#{html_escape(value)}")
-            end
-          end
+          cleaned_options = convert_booleans(options.stringify_keys.reject {|key, value| value.nil?})
+          ' ' + cleaned_options.map {|key, value| %(#{key}="#{html_escape(value.to_s)}")}.sort * ' ' unless cleaned_options.empty?
+        end
+
+        def convert_booleans(options)
+          %w( disabled readonly multiple ).each { |a| boolean_attribute(options, a) }
+          options
+        end
+
+        def boolean_attribute(options, attribute)
+          options[attribute] ? options[attribute] = attribute : options.delete(attribute)
         end
     end
   end

actionpack/test/template/tag_helper_test.rb

@@ -24,6 +24,11 @@ class TagHelperTest < Test::Unit::TestCase
     assert_equal "<p included=\"\" />", tag("p", :included => '')
   end
 
+  def test_tag_options_converts_boolean_option
+    assert_equal '<p disabled="disabled" multiple="multiple" readonly="readonly" />',
+      tag("p", :disabled => true, :multiple => true, :readonly => true)
+  end
+
   def test_content_tag
     assert_equal "<a href=\"create\">Create</a>", content_tag("a", "Create", "href" => "create")
     assert_equal content_tag("a", "Create", "href" => "create"),