Sending :id => nil to form helpers now properly omits the "id" html element [#4559 state:resolved]

Signed-off-by: José Valim <jose.valim@gmail.com>

2 files changed, 118 insertions, 5 deletions

diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index 932e9e2f95..414a5d4cd9 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -859,7 +859,13 @@ module ActionView
         options = options.stringify_keys
         tag_value = options.delete("value")
         name_and_id = options.dup
-        name_and_id["id"] = name_and_id["for"]
+
+        if name_and_id["for"]
+          name_and_id["id"] = name_and_id["for"]
+        else
+          name_and_id.delete("id")
+        end
+
         add_default_name_and_id_for_value(tag_value, name_and_id)
         options.delete("index")
         options["for"] ||= name_and_id["id"]
@@ -1027,7 +1033,7 @@ module ActionView
             pretty_tag_value = tag_value.to_s.gsub(/\s/, "_").gsub(/\W/, "").downcase
             specified_id = options["id"]
             add_default_name_and_id(options)
-            options["id"] += "_#{pretty_tag_value}" unless specified_id
+            options["id"] += "_#{pretty_tag_value}" if specified_id.blank? && options["id"].present?
           else
             add_default_name_and_id(options)
           end
@@ -1036,14 +1042,14 @@ module ActionView
         def add_default_name_and_id(options)
           if options.has_key?("index")
             options["name"] ||= tag_name_with_index(options["index"])
-            options["id"]   ||= tag_id_with_index(options["index"])
+            options["id"] = options.fetch("id", tag_id_with_index(options["index"]))
             options.delete("index")
           elsif defined?(@auto_index)
             options["name"] ||= tag_name_with_index(@auto_index)
-            options["id"]   ||= tag_id_with_index(@auto_index)
+            options["id"] = options.fetch("id", tag_id_with_index(@auto_index))
           else
             options["name"] ||= tag_name + (options.has_key?('multiple') ? '[]' : '')
-            options["id"]   ||= tag_id
+            options["id"] = options.fetch("id", tag_id)
           end
         end
 
diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb
index da2adc9869..3893d152a2 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -423,6 +423,90 @@ class FormHelperTest < ActionView::TestCase
                  check_box("post", "secret", :id => "i mean it")
   end
 
+  def test_nil_id
+    assert_dom_equal(
+      '<input name="post[title]" size="30" type="text" value="Hello World" />', text_field("post", "title", "id" => nil)
+    )
+    assert_dom_equal(
+      '<textarea cols="40" name="post[body]" rows="20">Back to the hill and over it again!</textarea>',
+      text_area("post", "body", "id" => nil)
+    )
+    assert_dom_equal(
+      '<input name="post[secret]" type="hidden" value="0" /><input checked="checked" name="post[secret]" type="checkbox" value="1" />',
+      check_box("post", "secret", "id" => nil)
+    )
+    assert_dom_equal(
+      '<input type="radio" name="post[secret]" value="0" />',
+      radio_button("post", "secret", "0", "id" => nil)
+    )
+    assert_dom_equal(
+      '<select name="post[secret]"></select>',
+      select("post", "secret", [], {}, "id" => nil)
+    )
+    assert_dom_equal text_field("post", "title", "id" => nil),
+                 text_field("post", "title", :id => nil)
+    assert_dom_equal text_area("post", "body", "id" => nil),
+                 text_area("post", "body", :id => nil)
+    assert_dom_equal check_box("post", "secret", "id" => nil),
+                 check_box("post", "secret", :id => nil)
+    assert_dom_equal radio_button("post", "secret", "id" => nil),
+                 radio_button("post", "secret", :id => nil)
+  end
+
+  def test_index
+    assert_dom_equal(
+      '<input name="post[5][title]" size="30" id="post_5_title" type="text" value="Hello World" />',
+      text_field("post", "title", "index" => 5)
+    )
+    assert_dom_equal(
+      '<textarea cols="40" name="post[5][body]" id="post_5_body" rows="20">Back to the hill and over it again!</textarea>',
+      text_area("post", "body", "index" => 5)
+    )
+    assert_dom_equal(
+      '<input name="post[5][secret]" type="hidden" value="0" /><input checked="checked" name="post[5][secret]" type="checkbox" value="1" id="post_5_secret" />',
+      check_box("post", "secret", "index" => 5)
+    )
+    assert_dom_equal(
+      text_field("post", "title", "index" => 5),
+      text_field("post", "title", "index" => 5)
+    )
+    assert_dom_equal(
+      text_area("post", "body", "index" => 5),
+      text_area("post", "body", "index" => 5)
+    )
+    assert_dom_equal(
+      check_box("post", "secret", "index" => 5),
+      check_box("post", "secret", "index" => 5)
+    )
+  end
+
+  def test_index_with_nil_id
+    assert_dom_equal(
+      '<input name="post[5][title]" size="30" type="text" value="Hello World" />',
+      text_field("post", "title", "index" => 5, 'id' => nil)
+    )
+    assert_dom_equal(
+      '<textarea cols="40" name="post[5][body]" rows="20">Back to the hill and over it again!</textarea>',
+      text_area("post", "body", "index" => 5, 'id' => nil)
+    )
+    assert_dom_equal(
+      '<input name="post[5][secret]" type="hidden" value="0" /><input checked="checked" name="post[5][secret]" type="checkbox" value="1" />',
+      check_box("post", "secret", "index" => 5, 'id' => nil)
+    )
+    assert_dom_equal(
+      text_field("post", "title", "index" => 5, 'id' => nil),
+      text_field("post", "title", :index => 5, :id => nil)
+    )
+    assert_dom_equal(
+      text_area("post", "body", "index" => 5, 'id' => nil),
+      text_area("post", "body", :index => 5, :id => nil)
+    )
+    assert_dom_equal(
+      check_box("post", "secret", "index" => 5, 'id' => nil),
+      check_box("post", "secret", :index => 5, :id => nil)
+    )
+  end
+
   def test_auto_index
     pid = @post.id
     assert_dom_equal(
@@ -449,6 +533,29 @@ class FormHelperTest < ActionView::TestCase
     )
   end
 
+  def test_auto_index_with_nil_id
+    pid = @post.id
+    assert_dom_equal(
+      "<input name=\"post[#{pid}][title]\" size=\"30\" type=\"text\" value=\"Hello World\" />",
+      text_field("post[]","title", :id => nil)
+    )
+    assert_dom_equal(
+      "<textarea cols=\"40\" name=\"post[#{pid}][body]\" rows=\"20\">Back to the hill and over it again!</textarea>",
+      text_area("post[]", "body", :id => nil)
+    )
+    assert_dom_equal(
+      "<input name=\"post[#{pid}][secret]\" type=\"hidden\" value=\"0\" /><input checked=\"checked\" name=\"post[#{pid}][secret]\" type=\"checkbox\" value=\"1\" />",
+      check_box("post[]", "secret", :id => nil)
+    )
+   assert_dom_equal(
+"<input checked=\"checked\" name=\"post[#{pid}][title]\" type=\"radio\" value=\"Hello World\" />",
+      radio_button("post[]", "title", "Hello World", :id => nil)
+    )
+    assert_dom_equal("<input name=\"post[#{pid}][title]\" type=\"radio\" value=\"Goodbye World\" />",
+      radio_button("post[]", "title", "Goodbye World", :id => nil)
+    )
+  end
+
   def test_form_for
     assert_deprecated do
       form_for(:post, @post, :html => { :id => 'create-post' }) do |f|