Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Broken escape_javascript after SafeBuffer modifications in rails 3.0.8 #1553

Closed
morgoth opened this Issue · 10 comments

7 participants

@morgoth

After this commit: 53a2c0b
(in rails 3-0-stable branch and present in rails 3.0.8)
javascript_escape connected with rendering template is not working correctly.

<%= escape_javascript(render("partial")) %>

Partial content:

<span> Topics </span>

Results:
rails 3.0.8:

"<span> Topics span>"

rails 3.0.7:

"<span> Topics <\\/span>\\n"

You can play with this bug using sample app: https://github.com/morgoth/escape_javascript_bug

@morgoth

related to #1555

@josevalim
Owner

escape_javascript needs to be fixed to not use $1.

@sekrett

Same problem here

@tardate

@josevalim that's what we're doing as a monkey-patch to get by for now, however it means gsub's behaviour remains changed from ruby core doc.. not too comfortable with that. I'm trying to find a good fix to suggest on 1555.

@josevalim
Owner

@tardate I don't think we can leave gsub intact. If it cannot work as in core, we will probably remove it straight away. that's why I suggest to change escape_javascript to fix the issue differently. :)

@starpeak

As a quick workaround you can write an initializer monkey-patching escape_javascript:

module ActionView::Helpers::JavaScriptHelper
  def escape_javascript_with_workaround(javascript)
    escape_javascript_without_workaround(javascript.try(:to_str)).html_safe
  end

  alias_method_chain :escape_javascript, :workaround
end
@madwire

same issue on 3.1.0.rc2

@josevalim
Owner

Fixed on #1558.

@josevalim josevalim closed this
@diogui

Hi. I'm just starting wiht rails 3.0.8. and I came across with the same problem ... Starpeak where should I put that monkey-patching escape_javascript ? Thanks for your help and Keep on the good work guys ! Amazing chages from Rails 1 to now . Rails is getting better and better :)

@diogui

I've update to the 3.0.9 version :) Wroking perfect :)

@ohammersmith ohammersmith referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.