This project is archived and is in readonly mode.

#5658 ✓resolved
Aaron

Layout option is ignored when partial renders a nested partial.

Reported by Aaron | September 18th, 2010 @ 11:59 PM

When a partial with a sub partial is rendered via a page.replace_html content, the layout option is ignored.

withdrawals_controller

  def new
    @withdrawal = Withdrawal.new

    respond_with(@withdrawal) do |format|
      format.js do
        render(:update) do |page|
          page.replace_html 'content', :partial => "new", :layout => "layouts/wallet"
        end
      end
    end
  end

_new.html.erb

<%= render :partial => 'layouts/errors', :locals => {:target => @withdrawal} %>
<%= form_for @withdrawal do |f| %>
  <p><strong>You have <%= number_to_currency(current_user.wallet.balance) %> available for withdrawal.</strong></p>
  <div class="input_con">
    <strong>Amount:</strong>
    <%= f.text_field :amount, :class => 'txt_input' %>
  </div>
  <div class="input_con">
    <strong>PayPal email address:</strong>
    <%= f.text_field :email, :class => 'txt_input large' %>
  </div>
  <%= submit_tag "Continue", :class => 'submit' %>
<% end %>

_wallet.html.erb

<% content_for :internal_content do %>
    <% @is_wallet = true %>
    <%= yield(:paging_content) %>
    
    <% if (params[:id] && ['deposits', 'withdrawals'].first {|contr| current_page?(:controller => contr, :action => :show, :id => params[:id])} ) %>
      <%= link_to '<< Back to wallet history', transactions_path, :class => 'f_right green' %>
    <% else %>
      <div id="submenu">
          <%= link_to('Deposit', new_deposit_path, :class => (current_page?(:controller => :deposits, :action => 'new') ? 'active' : ''), :remote => true) %> |
          <%= link_to('Withdraw', new_withdrawal_path, :class => (current_page?(:controller => :withdrawals, :action => 'new') ? 'active' : ''), :remote => true) %> |
          <%= link_to('History', transactions_path, :class => (current_page?(:controller => :transactions, :action => 'index') ? 'active' : ''), :remote => true) %>
      </div>
    <% end %>
    <%= yield %>
  <% end %>
  <%= render :file => 'layouts/_internal' if request.xhr? %>
Started GET "/withdrawals/new" for 127.0.0.1 at 2010-09-18 16:34:05 -0600
  Processing by WithdrawalsController#new as JS
Rendered layouts/_errors.html.erb (0.5ms)
  User Load (0.2ms)  SELECT `users`.* FROM `users` WHERE (`users`.`id` = 2) LIMIT 1
  CACHE (0.0ms)  SELECT `users`.* FROM `users` WHERE (`users`.`id` = 2) LIMIT 1
  Wallet Load (0.3ms)  SELECT `wallets`.* FROM `wallets` WHERE (`wallets`.user_id = 2) LIMIT 1
  SQL (0.3ms)  SELECT SUM(`transactions`.`amount`) AS sum_id FROM `transactions` WHERE (`transactions`.`type` = 'Deposit') AND (`transactions`.wallet_id = 679143898)
  SQL (0.3ms)  SELECT SUM(`transactions`.`amount`) AS sum_id FROM `transactions` WHERE (`transactions`.`type` = 'Withdrawal') AND (`transactions`.wallet_id = 679143898)
Rendered withdrawals/_new.html.erb (173.1ms)
Completed 200 OK in 237ms (Views: 175.6ms | ActiveRecord: 1.2ms)

If we remove the errors partial from new partial and run the exact same request, the _internal partial is then rendered correctly.

Started GET "/withdrawals/new" for 127.0.0.1 at 2010-09-18 16:36:30 -0600
  Processing by WithdrawalsController#new as JS
  User Load (0.3ms)  SELECT `users`.* FROM `users` WHERE (`users`.`id` = 2) LIMIT 1
  CACHE (0.0ms)  SELECT `users`.* FROM `users` WHERE (`users`.`id` = 2) LIMIT 1
  Wallet Load (0.3ms)  SELECT `wallets`.* FROM `wallets` WHERE (`wallets`.user_id = 2) LIMIT 1
  SQL (0.4ms)  SELECT SUM(`transactions`.`amount`) AS sum_id FROM `transactions` WHERE (`transactions`.`type` = 'Deposit') AND (`transactions`.wallet_id = 679143898)
  SQL (0.3ms)  SELECT SUM(`transactions`.`amount`) AS sum_id FROM `transactions` WHERE (`transactions`.`type` = 'Withdrawal') AND (`transactions`.wallet_id = 679143898)
Rendered withdrawals/_new.html.erb (184.6ms)
Rendered layouts/_internal.html.erb (1.8ms)
Completed 200 OK in 224ms (Views: 195.3ms | ActiveRecord: 1.3ms)

As you can see the _internal partial is rendered. This was tested with other simple partial that just had basic text in them, same result.

Comments and changes to this ticket

  • Aaron

    Aaron September 19th, 2010 @ 12:22 AM

    I should mention, this is Rails 3.0.0.

  • Aaron

    Aaron September 19th, 2010 @ 04:07 PM

    Some further information. Replacing

    <%= render :partial => 'layouts/errors', :locals => {:target => @withdrawal} %>
    

    with a the simplest partial, one that only renders a string and doesn't pass locals, results in the same problem. However, the following properly renders as expected:

    <%= render :text => 'some string' %>
    
  • Aaron

    Aaron September 21st, 2010 @ 04:38 AM

    OK, I thought I would try to clean up the example and get rid of a lot of noise from my last example. Here's a simplified version (on a new rails 3.0 project):

    MyController.rb

    class MyController < ApplicationController
      respond_to :html, :js
    
      def show
        respond_with do |format|
          format.js do
            render(:update) do |page|
              page.replace_html 'content', :partial => "show", :layout => "layouts/jspartial"
            end
          end
          format.html
        end
      end
    end
    

    _show.html.erb

    ---- SHOW PARTIAL
    

    layouts/_errors.html.erb

    ---SHOULD SHOW ERRORS---
    <br>
    

    layouts/_jspartial.html.erb

    <%= yield %>
    
    <br>
    -- JS PARTIAL END
    

    show.html.erb

    -- SHOW TEMPLATE START
    
    <br>
    <%= render :partial => "show" %>
    <br>
    
    -- SHOW TEMPLATE END
    
    <br>
    <%= link_to('Get content', my_path(1), :remote => true) %>
    
    <div id="content">
    </div>
    
  • Aaron

    Aaron September 21st, 2010 @ 04:44 AM

    Sorry everyone, not having much luck with entering ticket information.

    Anyway...this is a continuation of the previous comment...

    Attachement 1 (1.png) illustrates what a regular GET request to show produces.

    Attachement 2 (2.png) illustrates what happens when you click the "Get content link"

    Everything works fine.

    However, if you modify _show.html.erb to include rendering of the partial:

    _show.html.erb

    <%= render :partial => 'layouts/errors' %>
    ---- SHOW PARTIAL
    

    You'll see that when you click the "Get content" link, the results are similar to what is in 3.png. You can see that the "_jspartial.html.erb" is ignored.

    Hope that is enough information.
    Thanks!

    (again, sorry for the multiple posts)

  • gnufied

    gnufied September 21st, 2010 @ 05:57 PM

    The original ticket is invalid and layout option is not ignored when using replace_html, verified that.

    Second thing, when you are trying to use render :partial within a partial, but partial being rendered contains yield as first statement and hence that will never work. Layout files can't be rendered like partials.

    My vote,close this ticket as invalid.

  • gnufied
  • Aaron

    Aaron September 21st, 2010 @ 06:52 PM

    Just to make things clear. The layout option of replace_html IS ignored, but ONLY when the "<%= render :partial => 'layouts/errors' %>" is added to the '_show.html.erb'. If it is NOT added, the layout option of replace_html DOES work.

    layout is not always ignored, only in the case that I have outlined.

    Further, if, instead of adding "<%= render :partial => 'layouts/errors' %>" I add <%= render :text => "something" %> to "_show.html.erb", the layout option of replace_html DOES work.

    This may be expected behavior, but it seems odd.

    I've attached a simple demo application if you are still willing to investigate further.

    Thanks a lot for all your help!
    AR.

  • Shimon Young

    Shimon Young October 5th, 2010 @ 11:26 AM

    Just hit exact same bug in Rails 3.0.0

    Layout for parent partial is ignored if the partial contains a nested partial.

  • Shimon Young

    Shimon Young October 6th, 2010 @ 09:29 AM

    • Tag set to layout, partial

    OK. I think the fix for this is changing
    actionpack-3.00\lib\action_view\render\partials.rb
    from:

      def render
        identifier = ((@template = find_template) ? @template.identifier : @path)
    
        if @collection
          ActiveSupport::Notifications.instrument("render_collection.action_view",
            :identifier => identifier || "collection", :count => @collection.size) do
            render_collection
          end
        else
          content = ActiveSupport::Notifications.instrument("render_partial.action_view",
            :identifier => identifier) do
            render_partial
          end
          if !@block && (layout = @options[:layout])
            content = @view._render_layout(find_template(layout), @locals){ content }
          end
    
          content
        end
      end
    

    to:

      def render
        identifier = ((@template = find_template) ? @template.identifier : @path)
    
        if @collection
          ActiveSupport::Notifications.instrument("render_collection.action_view",
            :identifier => identifier || "collection", :count => @collection.size) do
            render_collection
          end
        else
          layout = @options[:layout]
          content = ActiveSupport::Notifications.instrument("render_partial.action_view",
            :identifier => identifier) do
            render_partial
          end
          if !@block && layout
            content = @view._render_layout(find_template(layout), @locals){ content }
          end
    
          content
        end
      end
    

    ...because the PartialRenderer object is shared throughout the entire render and the @options have been overwritten by any nested partials by the time it returns from render_partial. This way should keep the relevant @options in tact with recursion.

    Disclaimer: I've never even looked at the Rails code before now.

  • Shimon Young

    Shimon Young October 6th, 2010 @ 09:59 AM

    Actually, make that

    def render
      identifier = ((@template = find_template) ? @template.identifier : @path)
    
      if @collection
        ActiveSupport::Notifications.instrument("render_collection.action_view",
          :identifier => identifier || "collection", :count => @collection.size) do
          render_collection
        end
      else
        layout = @options[:layout]
        locals = @locals
        content = ActiveSupport::Notifications.instrument("render_partial.action_view",
          :identifier => identifier) do
          render_partial
        end
        if !@block && layout
          content = @view._render_layout(find_template(layout), locals){ content }
        end
    
        content
      end
    end
    

    so as to preserve the local vars also.

  • Shimon Young

    Shimon Young October 6th, 2010 @ 11:42 AM

    related to this, it seems the following fix is also needed in partials.rb if using a block with the layout

    def render_partial(object = @object)
      locals, view, template = @locals, @view, @template
    
      object ||= locals[template.variable_name]
      locals[@options[:as] || template.variable_name] = object
      block = @block
      template.render(view, locals) do |*name|
        view._layout_for(*name, &block)
      end
    
    end
    

    Again, without this fix, @block gets erased when rendering a nested partial

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:54 PM

    • Tag cleared.
    • Importance changed from “” to “Low”

    Automatic cleanup of spam.

  • Sebastian Henke

    Sebastian Henke October 21st, 2010 @ 01:05 AM

    • Tag set to partial nesting layout render block enclosed

    Hi,

    I hit on the same/similar problem. When nesting partials (blocks), always the layout of the deepest nested partial (block) is selected as layout for all enclosing partials.
    I tried it with 3.0.0, 3.0.1 and the current git master 3.1.0.beta. The Shimon's fix didn't work for me (in 3.0.1).
    Perhaps the priority of this bug should be increased as the missing layout functionality breaks DRY :/
    I hope to find some time to prepare some test cases to demonstrate more precisely the issue.

    Basti

  • Ben Askins

    Ben Askins October 28th, 2010 @ 04:45 AM

    • Tag changed from partial nesting layout render block enclosed to nested partial, partial layout, rails 3.0.0, layout, partial, render

    Seeing the same issue as described by Shimon and Sebastian. The :layout option on a partial is ignored when the partial being rendered, renders another partial.

    Rails 3.0.0, Rails 3.0.1

    cheers,
    Ben

  • Jeff Kreeftmeijer

    Jeff Kreeftmeijer November 1st, 2010 @ 05:05 PM

    • Tag cleared.

    Automatic cleanup of spam.

  • Carlos Antonio da Silva

    Carlos Antonio da Silva November 11th, 2010 @ 01:57 PM

    • Tag set to action_view, layout, partial, patch, render

    I got into the same issue, actually there are two different issues: one is when you render a layout giving a block, and the layout contains partials and then yield; and other is the render partial with nested partials and layout, but without block, as described here. I ended up with similar fixes as @Shimon, before finding this issue, and worked on some patches including tests:

    https://github.com/carlosantoniodasilva/rails/tree/render_layout_fix

  • Carlos Antonio da Silva

    Carlos Antonio da Silva November 11th, 2010 @ 01:58 PM

    • Title changed from “Layout option in replace_html is ignored when partial renders a sub partial.” to “Layout option is ignored when partial renders a nested partial.”

    I got into the same issue, actually there are two different issues: one is when you render a layout giving a block, and the layout contains partials and then yield; and other is the render partial with nested partials and layout, but without block, as described here. I ended up with similar fixes as @Shimon, before finding this issue, and worked on some patches including tests:

    https://github.com/carlosantoniodasilva/rails/tree/render_layout_fix

  • José Valim

    José Valim November 11th, 2010 @ 03:35 PM

    • State changed from “new” to “resolved”

    This has been fixed on 3-0-stable and master.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Pages