Merge pull request rails#15654 from pdg137/master

In tag helper, honor html_safe on arrays; also make safe_join more similar to Array.join

Author: matthewd

actionview/CHANGELOG.md

@@ -1,3 +1,13 @@
+*   Flatten the array parameter in `safe_join`, so it behaves consistently with
+    `Array#join`.
+
+    *Paul Grayson*
+
+*   Honor `html_safe` on array elements in tag values, as we do for plain string
+    values.
+
+    *Paul Grayson*
+
 *   Add `ActionView::Template::Handler.unregister_template_handler`.
 
     It performs the opposite of `ActionView::Template::Handler.register_template_handler`.

actionview/lib/action_view/helpers/output_safety_helper.rb

@@ -18,9 +18,9 @@ def raw(stringish)
       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 @@ def raw(stringish)
       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

actionview/lib/action_view/helpers/tag_helper.rb

@@ -173,8 +173,11 @@ def boolean_tag_option(key)
         end
 
         def tag_option(key, value, escape)
-          value = value.join(" ") if value.is_a?(Array)
-          value = ERB::Util.unwrapped_html_escape(value) if escape
+          if value.is_a?(Array)
+            value = escape ? safe_join(value, " ") : value.join(" ")
+          else
+            value = escape ? ERB::Util.unwrapped_html_escape(value) : value
+          end
           %(#{key}="#{value}")
         end
     end

actionview/test/template/output_safety_helper_test.rb

@@ -25,4 +25,11 @@ def setup
     assert_equal "<p>foo</p><br /><p>bar</p>", joined
   end
 
-end
+  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

actionview/test/template/tag_helper_test.rb

@@ -80,11 +80,27 @@ def test_content_tag_with_escaped_array_class
 
     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 @@ def test_tag_honors_html_safe_for_param_values
     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)