Fix rendering of HTML partials inside JS templates [#4197 status:resolved]

actionpack/lib/action_view/render/layouts.rb

@@ -43,10 +43,16 @@ def _layout_for(name = nil, &block) #:nodoc:
     # This is the method which actually finds the layout using details in the lookup
     # context object. If no layout is found, it checkes if at least a layout with
     # the given name exists across all details before raising the error.
-    def find_layout(layout) #:nodoc:
+    #
+    # If self.formats contains several formats, just the first one is considered in
+    # the layout lookup.
+    def find_layout(layout)
       begin
-        layout =~ /^\// ?
-          with_fallbacks { find_template(layout) } : find_template(layout)
+        if formats.size == 1
+          _find_layout(layout)
+        else
+          update_details(:formats => self.formats[0,1]){ _find_layout(layout) }
+        end
       rescue ActionView::MissingTemplate => e
         update_details(:formats => nil) do
           raise unless template_exists?(layout)
@@ -54,6 +60,11 @@ def find_layout(layout) #:nodoc:
       end
     end
 
+    def _find_layout(layout) #:nodoc:
+      layout =~ /^\// ?
+        with_fallbacks { find_template(layout) } : find_template(layout)
+    end
+
     # Contains the logic that actually renders the layout.
     def _render_layout(layout, locals, &block) #:nodoc:
       layout.render(self, locals){ |*name| _layout_for(*name, &block) }

actionpack/lib/action_view/template.rb

@@ -31,7 +31,9 @@ def initialize(source, identifier, handler, details)
       format    = details[:format]
       format  ||= handler.default_format.to_sym if handler.respond_to?(:default_format)
       format  ||= :html
+
       @formats  = [format.to_sym]
+      @formats << :html if @formats.first == :js
     end
 
     def render(view, locals, &block)

actionpack/test/controller/new_base/render_rjs_test.rb

@@ -5,8 +5,10 @@ class BasicController < ActionController::Base
     self.view_paths = [ActionView::FixtureResolver.new(
       "render_rjs/basic/index.js.rjs"          => "page[:customer].replace_html render(:partial => 'customer')",
       "render_rjs/basic/index_html.js.rjs"     => "page[:customer].replace_html :partial => 'customer'",
+      "render_rjs/basic/index_no_js.js.rjs"    => "page[:developer].replace_html render(:partial => 'developer')",
       "render_rjs/basic/_customer.js.erb"      => "JS Partial",
       "render_rjs/basic/_customer.html.erb"    => "HTML Partial",
+      "render_rjs/basic/_developer.html.erb"   => "HTML Partial",
       "render_rjs/basic/index_locale.js.rjs"   => "page[:customer].replace_html :partial => 'customer'",
       "render_rjs/basic/_customer.da.html.erb" => "Danish HTML Partial",
       "render_rjs/basic/_customer.da.js.erb"   => "Danish JS Partial"
@@ -37,6 +39,11 @@ def teardown
       assert_response("$(\"customer\").update(\"JS Partial\");")
     end
 
+    test "rendering a partial in an RJS template should pick the HTML one if no JS is available" do
+      get :index_no_js, "format" => "js"
+      assert_response("$(\"developer\").update(\"HTML Partial\");")
+    end
+
     test "replacing an element with a partial in an RJS template should pick the HTML template over the JS one" do
       get :index_html, "format" => "js"
       assert_response("$(\"customer\").update(\"HTML Partial\");")