Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Changed translate helper so that it doesn’t mark every translation as…

… safe HTML. Only keys with a "_html" suffix and keys named "html" are considered to be safe HTML. All other translations are left untouched.

Signed-off-by: David Heinemeier Hansson <david@loudthinking.com>
  • Loading branch information...
commit 5208cc3cf5d54720512ff50c2b97e9f4369aaa27 1 parent 0ab2ba3
Craig Davey authored dhh committed
View
5 actionpack/CHANGELOG
@@ -1,3 +1,8 @@
+*Rails 3.0.0 [beta 4/release candidate] (unreleased)*
+
+* Changed translate helper so that it doesn’t mark every translation as safe HTML. Only keys with a "_html" suffix and keys named "html" are considered to be safe HTML. All other translations are left untouched. [Craig Davey]
+
+
*Rails 3.0.0 [beta 3] (April 13th, 2010)*
* New option :as added to form_for allows to change the object name. The old <% form_for :client, @post %> becomes <% form_for @post, :as => :client %> [spastorino]
View
26 actionpack/lib/action_view/helpers/translation_helper.rb
@@ -3,17 +3,28 @@
module ActionView
module Helpers
module TranslationHelper
- # Delegates to I18n#translate but also performs two additional functions. First, it'll catch MissingTranslationData exceptions
+ # Delegates to I18n#translate but also performs three additional functions. First, it'll catch MissingTranslationData exceptions
# and turn them into inline spans that contains the missing key, such that you can see in a view what is missing where.
#
# Second, it'll scope the key by the current partial if the key starts with a period. So if you call translate(".foo") from the
# people/index.html.erb template, you'll actually be calling I18n.translate("people.index.foo"). This makes it less repetitive
# to translate many keys within the same partials and gives you a simple framework for scoping them consistently. If you don't
# prepend the key with a period, nothing is converted.
+ #
+ # Third, it’ll mark the translation as safe HTML if the key has the suffix "_html" or the last element of the key is the word
+ # "html". For example, calling translate("footer_html") or translate("footer.html") will return a safe HTML string that won’t
+ # be escaped by other HTML helper methods. This naming convention helps to identify translations that include HTML tags so that
+ # you know what kind of output to expect when you call translate in a template.
+
def translate(key, options = {})
options[:raise] = true
translation = I18n.translate(scope_key_by_partial(key), options)
- (translation.respond_to?(:join) ? translation.join : translation).html_safe
+ translation = (translation.respond_to?(:join) ? translation.join : translation)
+ if html_safe_translation_key? key
+ translation.html_safe
+ else
+ translation
+ end
rescue I18n::MissingTranslationData => e
keys = I18n.normalize_keys(e.locale, e.key, e.options[:scope])
content_tag('span', keys.join(', '), :class => 'translation_missing')
@@ -27,7 +38,7 @@ def localize(*args)
alias :l :localize
private
-
+
def scope_key_by_partial(key)
strkey = key.respond_to?(:join) ? key.join : key.to_s
if strkey.first == "."
@@ -40,6 +51,15 @@ def scope_key_by_partial(key)
key
end
end
+
+ def html_safe_translation_key?(key)
+ last_key = if key.is_a? Array
+ key.last
+ else
+ key.to_s.split('.').last
+ end
+ (last_key == "html") || (last_key.ends_with? "_html")
+ end
end
end
end
View
2  actionpack/test/fixtures/test/array_translation.erb
@@ -1 +1 @@
-<%= t(['foo', 'bar']) %>
+<%= t(['foo', 'bar', 'html']) %>
View
17 actionpack/test/template/translation_helper_test.rb
@@ -25,7 +25,7 @@ def test_translation_of_an_array
def test_translation_of_an_array_with_html
expected = '<a href="#">foo</a><a href="#">bar</a>'
- I18n.expects(:translate).with(["foo", "bar"], :raise => true).returns(['<a href="#">foo</a>', '<a href="#">bar</a>'])
+ I18n.expects(:translate).with(["foo", "bar", "html"], :raise => true).returns(['<a href="#">foo</a>', '<a href="#">bar</a>'])
@view = ActionView::Base.new(ActionController::Base.view_paths, {})
assert_equal expected, @view.render(:file => "test/array_translation")
end
@@ -47,4 +47,19 @@ def test_scoping_by_partial_of_an_array
@view = ActionView::Base.new(ActionController::Base.view_paths, {})
assert_equal "foobar", @view.render(:file => "test/scoped_array_translation")
end
+
+ def test_translate_does_not_mark_plain_text_as_safe_html
+ I18n.expects(:translate).with("hello", :raise => true).returns("Hello World")
+ assert_equal false, translate("hello").html_safe?
+ end
+
+ def test_translate_marks_translations_named_html_as_safe_html
+ I18n.expects(:translate).with("html", :raise => true).returns("<a>Hello World</a>")
+ assert translate("html").html_safe?
+ end
+
+ def test_translate_marks_translations_with_a_html_suffix_as_safe_html
+ I18n.expects(:translate).with("hello_html", :raise => true).returns("<a>Hello World</a>")
+ assert translate("hello_html").html_safe?
+ end
end

4 comments on commit 5208cc3

@rasmusrn

Have you considered this situation:

# we have this translation: en.welcome_message = 'Hello %{user}'
t 'welcome_message', :user => link_to(user.name, user)

Here we have no html in the translation itself, but are still required to name it welcome_html for it to work. Doesn't this seem wrong to you?

@KieranP

I agree with rasmusrn. It should follow the same rules are ERB:

t('welcome_message', :user => link_to(user.name, user))    => the user link is escaped, showing as html

t('welcome_message', :user => link_to(user.name, user).html_safe)    =>  the user link isn't escaped, parsing as an actual link

Ideally (if it doesn't already), link_to should return an html_safe by default. The title and url passed into it should be escaped within the link_to definition

@rasmusrn

Kieran, FYI have a look at the Rails Core Group thread I posted above. Koz makes a good point. The i18n module shouldn't be "html aware".

Please sign in to comment.
Something went wrong with that request. Please try again.