Provides standard layout lookup behavior for method and proc cases

When setting the layout either by referencing a method or supplying a
Proc there is no way to fall back to the default lookup behavior if
desired. This patch allows fallback to the layout lookup behavior when
returning nil from the proc or method.

4 files changed, 67 insertions, 5 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 5be7f34331..9507525042 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,25 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Ensure consistent fallback to the default layout lookup for layouts set
+    using symbols or procs that return nil.
+
+    All of the following layouts will result in the default layout lookup:
+
+        layout nil
+
+        layout proc { |c| nil }
+
+        layout :returns_nil
+
+        def returns_nil
+          nil
+        end
+
+    Previously symbols and procs which returned nil resulted in no layout which
+    differed from the `layout nil` behavior.
+
+    *Chris Nicola*
+
 *   Fix `respond_to` not using formats that have no block if all is present. *Michael Grosser*
 
 *   New applications use an encrypted session store by default.
diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb
index 91864f2a35..bac994496e 100644
--- a/actionpack/lib/abstract_controller/layouts.rb
+++ b/actionpack/lib/abstract_controller/layouts.rb
@@ -285,10 +285,9 @@ module AbstractController
         remove_possible_method(:_layout)
 
         prefixes    = _implied_layout_name =~ /\blayouts/ ? [] : ["layouts"]
+        default_behavior = "lookup_context.find_all('#{_implied_layout_name}', #{prefixes.inspect}).first || super"
         name_clause = if name
-          <<-RUBY
-            lookup_context.find_all("#{_implied_layout_name}", #{prefixes.inspect}).first || super
-          RUBY
+          default_behavior
         else
           <<-RUBY
             super
@@ -301,6 +300,7 @@ module AbstractController
           when Symbol
             <<-RUBY
               #{_layout}.tap do |layout|
+                return #{default_behavior} if layout.nil?
                 unless layout.is_a?(String) || !layout
                   raise ArgumentError, "Your layout method :#{_layout} returned \#{layout}. It " \
                     "should have returned a String, false, or nil"
@@ -308,8 +308,12 @@ module AbstractController
               end
             RUBY
           when Proc
-              define_method :_layout_from_proc, &_layout
-              _layout.arity == 0 ? "_layout_from_proc" : "_layout_from_proc(self)"
+            define_method :_layout_from_proc, &_layout
+            <<-RUBY
+              result = _layout_from_proc(#{_layout.arity == 0 ? '' : 'self'})
+              return #{default_behavior} if result.nil?
+              result
+            RUBY
           when false
             nil
           when true
diff --git a/actionpack/test/abstract/layouts_test.rb b/actionpack/test/abstract/layouts_test.rb
index 558a45b87f..70688d7267 100644
--- a/actionpack/test/abstract/layouts_test.rb
+++ b/actionpack/test/abstract/layouts_test.rb
@@ -79,6 +79,14 @@ module AbstractControllerTests
       end
     end
 
+    class WithProcReturningNil < Base
+      layout proc { |c| nil }
+
+      def index
+        render template: ActionView::Template::Text.new("Hello nil!")
+      end
+    end
+
     class WithZeroArityProc < Base
       layout proc { "overwrite" }
 
@@ -249,6 +257,12 @@ module AbstractControllerTests
         assert_equal "Overwrite Hello proc!", controller.response_body
       end
 
+      test "when layout is specified as a proc and the proc retuns nil, don't use a layout" do
+        controller = WithProcReturningNil.new
+        controller.process(:index)
+        assert_equal "Hello nil!", controller.response_body
+      end
+
       test "when layout is specified as a proc without parameters it works just the same" do
         controller = WithZeroArityProc.new
         controller.process(:index)
diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb
index 71bcfd664e..a61a1fe99d 100644
--- a/actionpack/test/controller/layout_test.rb
+++ b/actionpack/test/controller/layout_test.rb
@@ -94,6 +94,18 @@ class HasOwnLayoutController < LayoutTest
   layout 'item'
 end
 
+class HasNilLayoutSymbol < LayoutTest
+  layout :nilz
+
+  def nilz
+    nil
+  end
+end
+
+class HasNilLayoutProc < LayoutTest
+  layout proc { |c| nil }
+end
+
 class PrependsViewPathController < LayoutTest
   def hello
     prepend_view_path File.dirname(__FILE__) + '/../fixtures/layout_tests/alt/'
@@ -142,6 +154,18 @@ class LayoutSetInResponseTest < ActionController::TestCase
     assert_template :layout => "layouts/item"
   end
 
+  def test_layout_symbol_set_in_controller_returning_nil_falls_back_to_default
+    @controller = HasNilLayoutSymbol.new
+    get :hello
+    assert_template layout: "layouts/layout_test"
+  end
+
+  def test_layout_proc_set_in_controller_returning_nil_falls_back_to_default
+    @controller = HasNilLayoutProc.new
+    get :hello
+    assert_template layout: "layouts/layout_test"
+  end
+
   def test_layout_only_exception_when_included
     @controller = OnlyLayoutController.new
     get :hello