From 50d233414da2ba872eca356837fc12886474ddb8 Mon Sep 17 00:00:00 2001
From: Jeff Dean <jeff@zilkey.com>
Date: Sun, 9 May 2010 02:53:02 -0400
Subject: [PATCH] Sending :id => nil to form helpers now properly omits the "id" html element [#4559 state:resolved]

---
 actionpack/lib/action_view/helpers/form_helper.rb |   16 +++-
 actionpack/test/template/form_helper_test.rb      |   97 +++++++++++++++++++++
 2 files changed, 108 insertions(+), 5 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index 6e26ae6..6cc7a49 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -848,7 +848,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"]
@@ -1012,7 +1018,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
@@ -1021,14 +1027,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 2234fbe..402aeb8 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -419,6 +419,80 @@ 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 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)
+  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(
@@ -445,6 +519,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|
-- 
1.6.1.2