Do not call fields_for from form_for, to avoid instantiating two builders

Conflicts:
	actionpack/lib/action_view/helpers/form_helper.rb
	actionpack/test/template/form_helper_test.rb

2 files changed, 19 insertions, 9 deletions

diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index d00bad7608..7df74d96fb 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -331,9 +331,9 @@ module ActionView
       # In many cases you will want to wrap the above in another helper, so you
       # could do something like the following:
       #
-      #   def labelled_form_for(record_or_name_or_array, *args, &proc)
+      #   def labelled_form_for(record_or_name_or_array, *args, &block)
       #     options = args.extract_options!
-      #     form_for(record_or_name_or_array, *(args << options.merge(:builder => LabellingFormBuilder)), &proc)
+      #     form_for(record_or_name_or_array, *(args << options.merge(:builder => LabellingFormBuilder)), &block)
       #   end
       #
       # If you don't need to attach a form to a model instance, then check out
@@ -355,7 +355,7 @@ module ActionView
       #   <%= form_for @invoice, :url => external_url, :authenticity_token => false do |f|
       #     ...
       #   <% end %>
-      def form_for(record, options = {}, &proc)
+      def form_for(record, options = {}, &block)
         raise ArgumentError, "Missing block" unless block_given?
 
         options[:html] ||= {}
@@ -374,12 +374,10 @@ module ActionView
         options[:html][:method] = options.delete(:method) if options.has_key?(:method)
         options[:html][:authenticity_token] = options.delete(:authenticity_token)
 
-        builder = options[:parent_builder] = instantiate_builder(object_name, object, options, &proc)
-        fields_for = fields_for(object_name, object, options, &proc)
+        builder = options[:parent_builder] = instantiate_builder(object_name, object, options, &block)
+        output  = capture(builder, &block)
         default_options = builder.multipart? ? { :multipart => true } : {}
-        output = form_tag(options.delete(:url) || {}, default_options.merge!(options.delete(:html)))
-        output << fields_for
-        output.safe_concat('</form>')
+        form_tag(options.delete(:url) || {}, default_options.merge!(options.delete(:html))) { output }
       end
 
       def apply_form_for_options!(object_or_array, options) #:nodoc:
diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb
index 49a325af79..7b35424ec7 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -755,7 +755,6 @@ class FormHelperTest < ActionView::TestCase
     assert_dom_equal expected, output_buffer
   end
 
-
   def test_form_for_with_format
     form_for(@post, :format => :json, :html => { :id => "edit_post_123", :class => "edit_post" }) do |f|
       concat f.label(:title)
@@ -2217,6 +2216,19 @@ class FormHelperTest < ActionView::TestCase
     assert_equal "fields", output
   end
 
+  def test_form_for_only_instantiates_builder_once
+    initialization_count = 0
+    builder_class = Class.new(ActionView::Helpers::FormBuilder) do
+      define_method :initialize do |*args|
+        super(*args)
+        initialization_count += 1
+      end
+    end
+
+    form_for(@post, :builder => builder_class) { }
+    assert_equal 1, initialization_count, 'form builder instantiated more than once'
+  end
+
   protected
     def protect_against_forgery?
       false