From ef27bba63d93453dee26e2decfca80659f2da735 Mon Sep 17 00:00:00 2001
From: Chris Nicola <chnicola@gmail.com>
Date: Fri, 7 Dec 2012 15:55:54 -0800
Subject: [PATCH] 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.
---
 actionpack/CHANGELOG.md                       | 20 ++++++++++++++++
 actionpack/lib/abstract_controller/layouts.rb | 14 +++++++----
 actionpack/test/abstract/layouts_test.rb      | 14 +++++++++++
 actionpack/test/controller/layout_test.rb     | 24 +++++++++++++++++++
 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 @@ def _write_layout_method # :nodoc:
         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 @@ def _write_layout_method # :nodoc:
           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 @@ def _write_layout_method # :nodoc:
               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 @@ def index
       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 @@ class TestBase < ActiveSupport::TestCase
         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 @@ def test_layout_set_when_set_in_controller
     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