Corrected the html_safe implementation for Array. Moved the html safe version of join to its own method (safe_join) as not to degrade the performance of join for unrelated html_safe use. [#6298 state:resolved]

2 files changed, 35 insertions, 20 deletions

diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb
index 520aa4e67d..0c8fc20ea5 100644
--- a/activesupport/lib/active_support/core_ext/string/output_safety.rb
+++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -124,17 +124,32 @@ class String
 end
 
 class Array
-
-  alias_method :original_join, :join
-
-  def join(sep=$,)
+  # If the separator and all the items in the array are html safe
+  # then an html safe string is returned using <tt>Array#join</tt>,
+  # otherwise the result of <tt>Array#join</tt> is returned without
+  # marking it as html safe.
+  #
+  #   ["Mr", "Bojangles"].join.html_safe?
+  #   # => false
+  #
+  #   ["Mr".html_safe, "Bojangles".html_safe].join.html_safe?
+  #   # => true
+  #
+  def safe_join(sep=$,)
     sep ||= "".html_safe
-    str = original_join(sep)
+    str = join(sep)
     (sep.html_safe? && html_safe?) ? str.html_safe : str
   end
 
+  # Returns +true+ if all items in the array are html safe.
+  #
+  #   [""].html_safe?
+  #   # => false
+  #
+  #   ["".html_safe].html_safe?
+  #   # => true
+  #
   def html_safe?
-    self.detect {|e| !e.html_safe?}.nil?
+    detect { |e| !e.html_safe? }.nil?
   end
-
 end
diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb
index 41a23641d4..15e39a06c3 100644
--- a/activesupport/test/core_ext/string_ext_test.rb
+++ b/activesupport/test/core_ext/string_ext_test.rb
@@ -435,36 +435,36 @@ class OutputSafetyTest < ActiveSupport::TestCase
   end
 
   test "Joining safe elements without a separator is safe" do
-    array = 5.times.collect {"some string".html_safe}
-    assert array.join.html_safe?
+    array = 5.times.collect { "some string".html_safe }
+    assert array.safe_join.html_safe?
   end
 
   test "Joining safe elements with a safe separator is safe" do
-    array = 5.times.collect {"some string".html_safe}
-    assert array.join("-".html_safe).html_safe?
+    array = 5.times.collect { "some string".html_safe }
+    assert array.safe_join("-".html_safe).html_safe?
   end
 
   test "Joining safe elements with an unsafe separator is unsafe" do
-    array = 5.times.collect {"some string".html_safe}
-    assert_false array.join("-").html_safe?
+    array = 5.times.collect { "some string".html_safe }
+    assert !array.safe_join("-").html_safe?
   end
 
   test "Joining is unsafe if any element is unsafe even with a safe separator" do
-    array = 5.times.collect {"some string".html_safe}
+    array = 5.times.collect { "some string".html_safe }
     array << "some string"
-    assert_false array.join("-".html_safe).html_safe?
+    assert !array.safe_join("-".html_safe).html_safe?
   end
 
   test "Joining is unsafe if any element is unsafe and no separator is given" do
-    array = 5.times.collect {"some string".html_safe}
+    array = 5.times.collect { "some string".html_safe }
     array << "some string"
-    assert_false array.join.html_safe?
+    assert !array.safe_join.html_safe?
   end
 
   test "Joining is unsafe if any element is unsafe and the separator is unsafe" do
-    array = 5.times.collect {"some string".html_safe}
+    array = 5.times.collect { "some string".html_safe }
     array << "some string"
-    assert_false array.join("-").html_safe?
+    assert !array.safe_join("-").html_safe?
   end
 
   test "Array is safe if all elements are safe" do
@@ -475,7 +475,7 @@ class OutputSafetyTest < ActiveSupport::TestCase
   test "Array is unsafe if any element is unsafe" do
     array = 5.times.collect { "some string".html_safe }
     array << "some string"
-    assert_false array.html_safe?
+    assert !array.html_safe?
   end
 
   test 'emits normal string yaml' do