Merge pull request #8458 from lucisferre/improve-layout-override-fallback-behavior

Provides standard layout lookup behavior for method and proc cases

Conflicts:
	actionpack/CHANGELOG.md

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*
+
 *   Create `UpgradeLegacySignedCookieJar` to transparently upgrade existing signed
     cookies generated by Rails 3.x to avoid invalidating them when upgrading to Rails 4.x.
 
@@ -104,6 +124,7 @@
 
     *Thierry Zires*
 
+
 ## Rails 4.0.0.beta1 (February 25, 2013) ##
 
 *   Fix `respond_to` not using formats that have no block if all is present. *Michael Grosser*

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

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)

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