This project is archived and is in readonly mode.

#386 ✓ resolved
Nolan Eakins

Nested content_tag calls causes the layout to render multiple times.

Reported by Nolan Eakins | June 11th, 2008 @ 04:52 AM

I have a helper that uses #content_tag within a block passed to another call to #content_tag. Whenever my view calls this it restarts the render from the beginning causing my header to get shown twice.

In theory a layout such as:

header

<%= content_tag('p') { content_tag('b', 'hello') } %>

footer

should have "header" displayed twice.

Attached is a diff to the actionpack test suite with a case for nested content_tag calls. As expected, it fails.

Comments and changes to this ticket

  • Thomas Watson

    Thomas Watson June 12th, 2008 @ 06:18 PM

    Same problem here... all my views are rendered twice.

    During my debugging of this issue I tried to output the return value of a content_tag that where given a block. It seems to only return the empty tag without any content:

    header = content_tag :h1 do

    "Foobar"

    end

    puts header

    The above code should output "

    Foobar

    ", but outputs "

    "

  • Thomas Watson

    Thomas Watson June 12th, 2008 @ 06:21 PM

    Ups... The comment field did not escape my html tags as I expected... trying again:

    The above code should output "<h1>Foobar</h1>", but outputs "<h1></h1>"

  • Jeremy Kemper

    Jeremy Kemper June 12th, 2008 @ 09:52 PM

    • Milestone set to 2.1.1
    • State changed from “new” to “open”
    • Assigned user set to “Jeremy Kemper”

    This is because content_tag is supposed to behave differently depending on whether it's called within a helper method or within an erb view directly.

    In erb, you say <% content_tag :foo do %>...<% end %>. To be consistent, these should really be <%= content_tag :foo do %>...<% end %> instead, but erb doesn't support that. Argh.

    Anyway, I'm working on a fix.

  • Thomas Watson

    Thomas Watson June 13th, 2008 @ 02:26 PM

    As far as I can see (and please bear with me if I don't get it right - it's the first time I look into the ActionView source), the problem is that concat (_actionpack/lib/action_view/helpers/text_helper.rb:28_) relies on the return value of the output_buffer method. If the return value is either nil or false, concat will return the given string without doing anything (the call to concat could in this case technically be omitted). In all other situations it will push it to the existing @output_buffer.

    This would only work if we could expect @output_buffer to be nil if concat where called from a helper method inside another helper method, or a helper method inside a controller. But @output_buffer will always be initialized (and stay so) if a view has begun to be rendered.

    So there is two solutions as far as I can see:

    1. Either don't call concat unless your helper method is called directly from a view.
    2. Or replace the simple "if output_buffer" check inside the concat method with something that knows if it is called directly from a view

    Previously this was done by a call to _block_is_within_action_view?_ which would:

    def block_is_within_action_view?(block)
      eval("defined? _erbout", block.binding)
    end
    

    But since we don't pass the binding to the concat method anymore this cannot be done. I'm wonder if there is another to get the same info?

  • Jeremy Kemper

    Jeremy Kemper June 13th, 2008 @ 06:20 PM

    You're right on. I've been investigating workarounds that preserve the speedup and memory savings I got from this change, but we're between a rock and a hard place.

  • Thomas Watson

    Thomas Watson June 15th, 2008 @ 11:03 PM

    What was it that took up processor time and memory? Was it the check to verify if '_erbout' existsed on the binding or was it the push to 'ActionView::Base.erb_variable'? ... or both?

  • Jeremy Kemper

    Jeremy Kemper June 15th, 2008 @ 11:12 PM

    Passing Proc#binding ate a ton of memory. Having to slice _erbout instead of swapping in a new buffer ate a ton of memory and cpu time.

  • Jeremy Kemper
  • Jeremy Kemper

    Jeremy Kemper June 20th, 2008 @ 07:03 AM

    • State changed from “open” to “resolved”

    Fixed by 72f93b5

    I'm checking for an __in_erb_template sentinel now.

  • Dan Pickett

    Dan Pickett October 15th, 2008 @ 10:52 PM

    • Tag set to 2.1, actionpack, bug, content_tag, edge

    will this be a recommended utility (I notice block_is_within_action_view? is private)? I have a wrapper that takes a block that I'm trying to write.

    I'd like to use in both helpers and views. It would be great to have a mechanism like this. Is there something less memory intensive I should be using?

  • Dan Pickett

    Dan Pickett October 15th, 2008 @ 10:54 PM

    • Tag cleared.

    clearing tags

Create your profile

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

Tickets have moved to Github

The new ticket tracker is available at https://github.com/rails/rails/issues

Shared Ticket Bins

Attachments

Pages