Rails Ruby on Rails → 3.1

Comments and changes to this ticket

• You flagged this item as spam.
  

  Ben Hollis September 29th, 2010 @ 07:35 AM

  Oops, forgot to mention, this is on 3.0.0.

• You flagged this item as spam.
  

  Álvaro Bautista October 10th, 2010 @ 07:45 PM

  I run your example and I got the same result.

  In fact, it seems to happen when yield is called within any helper method. For instance, having the following helper:

  def wadus
    yield
  end
  

  and the following view:

  <% wadus do %>
    Hello
  <% end %>
  

  the returned string is:

  Hello Hello
  

  Looking at actionpack/lib/action_view/template/handlers/erb.rb and putting some debugger statements I've seen that the resulting code for the block is

  @output_buffer = ActionView::OutputBuffer.new;@output_buffer.append_if_string=  wadus do \n@output_buffer.safe_concat('  Hello\n'); end ;@output_buffer.to_s
  

  The string is concatenated to the output buffer twice, once through safe_concat and another time through append_if_string.

  I think the class ActionView::Template::Handlers::Erubis just extends Erubis::Eruby to use ActionView::OutputBuffer as the output buffer instead of regular strings (or whatever Erubis uses), so it may be just the way Erubis works.

  I've also run the code on 2.3.5 and the string is concatenated to the output buffer just once, so I guess ERB behaves differently in this case.

  I couldn't try in edge because of an annoying problem with arel dependency version.

• You flagged this item as spam.
  

  Álvaro Bautista October 10th, 2010 @ 11:27 PM

  I kind of found a workaround, it consists of wrapping the yield call with with_output_buffer:

  def wadus
    with_output_buffer { yield }
  end
  

  That implies that anyone writing helpers should know about this and wrap every yield statement to use an auxiliar buffer. Is this a good way of avoiding the double render in these cases?

  Would it be an acceptable solution to alias or extend the ActiveSupport::Benchmarkable#benchmark method in ActionView::Helpers?

  Doing something like:

  module ActionView::Helpers
    def benchmark(message = "Benchmarking", options = {})
      with_output_buffer { super(message, options) }
    end
  end
  

  I wouldn't feel very comfortable with having to change all my yield statements with something { yield }.

  I'm sure this issue must have already been discussed by core members or active committers, so I guess I'm just wandering.

• 

  Ryan Bigg October 19th, 2010 @ 08:23 AM

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

  Automatic cleanup of spam.

• 

  Santiago Pastorino February 2nd, 2011 @ 04:48 PM

  • State changed from “new” to “open”

  This issue has been automatically marked as stale because it has not been commented on for at least three months.

  The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

  Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

• 

  Santiago Pastorino February 2nd, 2011 @ 04:48 PM

  • State changed from “open” to “stale”
• 

  Aaron Patterson February 16th, 2011 @ 05:09 AM

  • State changed from “stale” to “open”
  • Assigned user set to “Aaron Patterson”

  Reopening. This is a real bug that exists in 3-0-stable but not master. O_O

• You flagged this item as spam.
  

  Ben Hollis February 16th, 2011 @ 08:43 AM

  I futzed around with git bisect, and it looks like ba52748d05da4f95a8f371d628af97b76644bdd3 was the commit that introduced the fix: https://github.com/rails/rails/commit/ba52748d05da4f95a8f371d628af9...

• You flagged this item as spam.
  

  Ben Hollis February 16th, 2011 @ 09:00 AM

  It appears that just applying the changes to actionpack/lib/action_view/template/handlers/erb.rb (deleting ActionView::OutputBuffer#append_if_string= and ActionView::Template::Handlers::Erubis#add_stmt) from that commit is enough to fix the bug. Looks like that'll get rid of the deprecation warning too, though.

• 

  Aaron Patterson February 25th, 2011 @ 12:43 AM

  • State changed from “open” to “wontfix”
  • Milestone set to “3.1”

  Alright, this bug exists because of backwards compatibility with rails 2.3. The problem is that rails 2.3 would concatenate strings with <% erb tags. If you examine the compiled ERb, you'll see:

  1. The body of the yield appends to the output.
     
  2. The return value of the append call is the value.
     
  3. The return value of the call is appended to the output.

  The best thing to do in Rails 3.0 is use capture, like this:

  def helper(&block)
    capture(&block)
  end
  

  Removing the backwards compat fixes this problem, so for now, please just use capture. Sorry! :-(

• You flagged this item as spam.
  

  Ben Hollis February 25th, 2011 @ 06:40 PM

  Hey Aaron, thanks for taking another look at this. I agree that new helpers should just use capture, but what about existing helpers (and methods that use yield but aren't view-specific), like https://github.com/rails/rails/blob/master/activesupport/lib/active...? Would the recommended usage for that be:
  

    <%= benchmark("Bug") do %>
      <% capture do %>
        Hello
      <% end %>
    <% end %>
  

  Would it maybe make sense to change that template helper to automatically add the capture to whenever we're using that backwards-compatibility mode?