In tag helper, honor html_safe on array parameters; also make safe_join more similar to Array.join by first calling flatten.

4 files changed, 51 insertions, 8 deletions

diff --git a/actionview/lib/action_view/helpers/output_safety_helper.rb b/actionview/lib/action_view/helpers/output_safety_helper.rb
index e1f40011c0..b0d9c7c7f9 100644
--- a/actionview/lib/action_view/helpers/output_safety_helper.rb
+++ b/actionview/lib/action_view/helpers/output_safety_helper.rb
@@ -18,9 +18,9 @@ module ActionView #:nodoc:
       end
 
       # This method returns a html safe string similar to what <tt>Array#join</tt>
-      # would return. All items in the array, including the supplied separator, are
-      # html escaped unless they are html safe, and the returned string is marked
-      # as html safe.
+      # would return. The array is flattened, and all items, including
+      # the supplied separator, are html escaped unless they are html
+      # safe, and the returned string is marked as html safe.
       #
       #   safe_join(["<p>foo</p>".html_safe, "<p>bar</p>"], "<br />")
       #   # => "<p>foo</p>&lt;br /&gt;&lt;p&gt;bar&lt;/p&gt;"
@@ -31,7 +31,7 @@ module ActionView #:nodoc:
       def safe_join(array, sep=$,)
         sep = ERB::Util.unwrapped_html_escape(sep)
 
-        array.map { |i| ERB::Util.unwrapped_html_escape(i) }.join(sep).html_safe
+        array.flatten.map! { |i| ERB::Util.unwrapped_html_escape(i) }.join(sep).html_safe
       end
     end
   end
diff --git a/actionview/lib/action_view/helpers/tag_helper.rb b/actionview/lib/action_view/helpers/tag_helper.rb
index 9b9ca7d60d..89a30d4b69 100644
--- a/actionview/lib/action_view/helpers/tag_helper.rb
+++ b/actionview/lib/action_view/helpers/tag_helper.rb
@@ -173,9 +173,21 @@ module ActionView
         end
 
         def tag_option(key, value, escape)
-          value = value.join(" ") if value.is_a?(Array)
-          value = ERB::Util.unwrapped_html_escape(value) if escape
-          %(#{key}="#{value}")
+          escaped_value = case value
+                          when Array
+                            if escape
+                              safe_join(value, " ")
+                            else
+                              value.join(" ")
+                            end
+                          else
+                            if escape
+                              ERB::Util.unwrapped_html_escape(value)
+                            else
+                              value
+                            end
+                          end
+          %(#{key}="#{escaped_value}")
         end
     end
   end
diff --git a/actionview/test/template/output_safety_helper_test.rb b/actionview/test/template/output_safety_helper_test.rb
index 76c71c9e6d..a1bf0e1a5f 100644
--- a/actionview/test/template/output_safety_helper_test.rb
+++ b/actionview/test/template/output_safety_helper_test.rb
@@ -25,4 +25,11 @@ class OutputSafetyHelperTest < ActionView::TestCase
     assert_equal "<p>foo</p><br /><p>bar</p>", joined
   end
 
-end
\ No newline at end of file
+  test "safe_join should work recursively similarly to Array.join" do
+    joined = safe_join(['a',['b','c']], ':')
+    assert_equal 'a:b:c', joined
+
+    joined = safe_join(['"a"',['<b>','<c>']], ' <br/> ')
+    assert_equal '&quot;a&quot; &lt;br/&gt; &lt;b&gt; &lt;br/&gt; &lt;c&gt;', joined
+  end
+end
diff --git a/actionview/test/template/tag_helper_test.rb b/actionview/test/template/tag_helper_test.rb
index fb016a52de..c78b6450f2 100644
--- a/actionview/test/template/tag_helper_test.rb
+++ b/actionview/test/template/tag_helper_test.rb
@@ -80,11 +80,27 @@ class TagHelperTest < ActionView::TestCase
 
     str = content_tag('p', "limelight", :class => ["song", "play"])
     assert_equal "<p class=\"song play\">limelight</p>", str
+
+    str = content_tag('p', "limelight", :class => ["song", ["play"]])
+    assert_equal "<p class=\"song play\">limelight</p>", str
   end
 
   def test_content_tag_with_unescaped_array_class
     str = content_tag('p', "limelight", {:class => ["song", "play>"]}, false)
     assert_equal "<p class=\"song play>\">limelight</p>", str
+
+    str = content_tag('p', "limelight", {:class => ["song", ["play>"]]}, false)
+    assert_equal "<p class=\"song play>\">limelight</p>", str
+  end
+
+  def test_content_tag_with_empty_array_class
+    str = content_tag('p', 'limelight', {:class => []})
+    assert_equal '<p class="">limelight</p>', str
+  end
+
+  def test_content_tag_with_unescaped_empty_array_class
+    str = content_tag('p', 'limelight', {:class => []}, false)
+    assert_equal '<p class="">limelight</p>', str
   end
 
   def test_content_tag_with_data_attributes
@@ -115,6 +131,14 @@ class TagHelperTest < ActionView::TestCase
     end
   end
 
+  def test_tag_honors_html_safe_with_escaped_array_class
+    str = tag('p', :class => ['song>', 'play>'.html_safe])
+    assert_equal '<p class="song&gt; play>" />', str
+
+    str = tag('p', :class => ['song>'.html_safe, 'play>'])
+    assert_equal '<p class="song> play&gt;" />', str
+  end
+
   def test_skip_invalid_escaped_attributes
     ['&1;', '&#1dfa3;', '& #123;'].each do |escaped|
       assert_equal %(<a href="#{escaped.gsub(/&/, '&amp;')}" />), tag('a', :href => escaped)