Merge pull request rails#54020 from flavorjones/flavorjones-fix-layout-locals

Action View: fix local variable access in layouts

Author: rafaelfranca

actionview/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Layouts have access to local variables passed to `render`.
+
+    This fixes #31680 which was a regression in Rails 5.1.
+
+    *Mike Dalessio*
+
 *   Argument errors related to strict locals in templates now raise an
     `ActionView::StrictLocalsError`, and all other argument errors are reraised as-is.
 

actionview/lib/action_view/layouts.rb

@@ -284,7 +284,7 @@ def _write_layout_method # :nodoc:
         silence_redefinition_of_method(:_layout)
 
         prefixes = /\blayouts/.match?(_implied_layout_name) ? [] : ["layouts"]
-        default_behavior = "lookup_context.find_all('#{_implied_layout_name}', #{prefixes.inspect}, false, [], { formats: formats }).first || super"
+        default_behavior = "lookup_context.find_all('#{_implied_layout_name}', #{prefixes.inspect}, false, keys, { formats: formats }).first || super"
         name_clause = if name
           default_behavior
         else
@@ -325,7 +325,7 @@ def _write_layout_method # :nodoc:
 
         class_eval <<-RUBY, __FILE__, __LINE__ + 1
           # frozen_string_literal: true
-          def _layout(lookup_context, formats)
+          def _layout(lookup_context, formats, keys)
             if _conditional_layout?
               #{layout_definition}
             else
@@ -389,8 +389,8 @@ def _layout_for_option(name)
       case name
       when String     then _normalize_layout(name)
       when Proc       then name
-      when true       then Proc.new { |lookup_context, formats| _default_layout(lookup_context, formats, true)  }
-      when :default   then Proc.new { |lookup_context, formats| _default_layout(lookup_context, formats, false) }
+      when true       then Proc.new { |lookup_context, formats, keys| _default_layout(lookup_context, formats, keys, true)  }
+      when :default   then Proc.new { |lookup_context, formats, keys| _default_layout(lookup_context, formats, keys, false) }
       when false, nil then nil
       else
         raise ArgumentError,
@@ -412,9 +412,9 @@ def _normalize_layout(value)
     #
     # ==== Returns
     # * <tt>template</tt> - The template object for the default layout (or +nil+)
-    def _default_layout(lookup_context, formats, require_layout = false)
+    def _default_layout(lookup_context, formats, keys, require_layout = false)
       begin
-        value = _layout(lookup_context, formats) if action_has_layout?
+        value = _layout(lookup_context, formats, keys) if action_has_layout?
       rescue NameError => e
         raise e, "Could not render layout: #{e.message}"
       end

actionview/lib/action_view/renderer/template_renderer.rb

@@ -99,14 +99,14 @@ def resolve_layout(layout, keys, formats)
             if layout.start_with?("/")
               raise ArgumentError, "Rendering layouts from an absolute path is not supported."
             else
-              @lookup_context.find_template(layout, nil, false, [], details)
+              @lookup_context.find_template(layout, nil, false, keys, details)
             end
           rescue ActionView::MissingTemplate
             all_details = @details.merge(formats: @lookup_context.default_formats)
-            raise unless template_exists?(layout, nil, false, [], **all_details)
+            raise unless template_exists?(layout, nil, false, keys, **all_details)
           end
         when Proc
-          resolve_layout(layout.call(@lookup_context, formats), keys, formats)
+          resolve_layout(layout.call(@lookup_context, formats, keys), keys, formats)
         else
           layout
         end

actionview/test/actionpack/abstract/layouts_test.rb

@@ -15,7 +15,7 @@ class Base < AbstractController::Base
       self.view_paths = [ActionView::FixtureResolver.new(
         "some/template.erb"             => "hello <%= foo %> bar",
         "layouts/hello.erb"             => "With String <%= yield %>",
-        "layouts/hello_locals.erb"      => "With String <%= yield %>",
+        "layouts/hello_locals.erb"      => "With String <%= foo %> <%= yield %>",
         "layouts/hello_override.erb"    => "With Override <%= yield %>",
         "layouts/overwrite.erb"         => "Overwrite <%= yield %>",
         "layouts/with_false_layout.erb" => "False Layout <%= yield %>",
@@ -41,6 +41,10 @@ class WithStringLocals < Base
       def index
         render template: "some/template", locals: { foo: "less than 3" }
       end
+
+      def overwrite_with_default
+        render layout: :default, template: "some/template", locals: { foo: "less than 3" }
+      end
     end
 
     class WithString < Base
@@ -279,10 +283,16 @@ class TestBase < ActiveSupport::TestCase
         assert_equal "Hello blank!", controller.response_body
       end
 
-      test "with locals" do
+      test "when layout is specified as a string, render with locals" do
         controller = WithStringLocals.new
         controller.process(:index)
-        assert_equal "With String hello less than 3 bar", controller.response_body
+        assert_equal "With String less than 3 hello less than 3 bar", controller.response_body
+      end
+
+      test "when layout is specified as a string, overwriting with :default, render with locals" do
+        controller = WithStringLocals.new
+        controller.process(:overwrite_with_default)
+        assert_equal "With String less than 3 hello less than 3 bar", controller.response_body
       end
 
       test "when layout is specified as a string, render with that layout" do