This project is archived and is in readonly mode.

#3856 ✓committed
Santiago Pastorino

[PATCH] More html_safe strings now use the safe_concat method

Reported by Santiago Pastorino | February 5th, 2010 @ 12:16 AM

Comments and changes to this ticket

  • Repository

    Repository February 5th, 2010 @ 09:29 PM

    • State changed from “new” to “committed”

    (from [e115eb097e3e5704b9861b6f2b92cc25d23de07c]) More html_safe strings now use the safe_concat method

    [#3856 state:committed]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
    http://github.com/rails/rails/commit/e115eb097e3e5704b9861b6f2b92cc...

  • korch

    korch April 2nd, 2010 @ 12:01 AM

    • Tag changed from 3.0pre, action_view, helpers, patch, performance to 2.3-stable, 3.0pre, action_view, helpers, patch, performance
    • Assigned user changed from “Yehuda Katz (wycats)” to “Santiago Pastorino”

    I think this commit might have broken something. I'm on 2.3-stable, freshly vendor'd from github, and I'm getting this error from any template calling form_for: "undefined method safe_concat' for "":String". It's pretty easy to recreate from a tosser app, and Rails own tests currently fail for this. I am using Haml to render the bare-bones form_for inside "new.html.haml", like everybody does, and this error occurs when using either haml stable or haml-edge.

    # gem list|grep -E 'rails|haml'
    # haml (2.2.22)
    # haml-edge (2.3.184)
    # rails (2.3.5)
    
    rails foobar
    cd foobar/vendor
    git clone git://github.com/rails/rails.git
    cd rails
    git checkout -b 2-3-stable origin/2-3-stable
    
    cd actionpack
    rake test_action_pack
    

    After fifty-something FormHelper errors for "Expected block to return true value", the test output fails:

    55) Error: test_safe_concat(TextHelperTest): NoMethodError: undefined method `safe_concat' for #
        /home/korch/safe_concat_broke/vendor/rails/actionpack/lib/action_controller/test_process.rb:511:in `method_missing'
        /home/korch/safe_concat_broke/vendor/rails/actionpack/lib/action_view/test_case.rb:158:in `method_missing'
        /home/korch/safe_concat_broke/vendor/rails/actionpack/test/template/text_helper_test.rb:27:in `test_safe_concat'
        /usr/local/ruby19/lib/ruby/gems/1.9.1/gems/mocha-0.9.8/lib/mocha/integration/mini_test/version_131_and_above.rb:26:in `run'
        /home/korch/safe_concat_broke/vendor/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:24:in `run'
    

    Looking at actionpack/lib/action_view/helpers/text_helper.rb, I see it is now calling out to safe_concat. I thought this was a method only for Rails 3, so I'm not sure why it's appearing in the 2.3-stable branch(2.3.6)?

    This safe_concat helper error goes away if I change line #29 in actionpack/lib/action_view/helpers/text_helper.rb like so:

    -        output_buffer.safe_concat(string)
    +  
    output_buffer.concat(string)

    Test case attached and I think I got it right, as this is my first time submitting to Rails.

  • Santiago Pastorino

    Santiago Pastorino April 2nd, 2010 @ 02:23 AM

    The test you mention aren't failing and your patch doesn't work and doesn't make your test pass.
    Can you send me another probe of a failure?.
    Thanks for helping.

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

Referenced by

Pages