option_groups_from_collection_for_select should return HTML-safe string [#4879 state:resolved]

Signed-off-by: Xavier Noria <fxn@hashref.com>

2 files changed, 13 insertions, 19 deletions

diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb
index fe71d2cdf7..e48580e0ad 100644
--- a/actionpack/lib/action_view/helpers/form_options_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -398,7 +398,7 @@ module ActionView
           options_for_select += "<optgroup label=\"#{html_escape(group_label_string)}\">"
           options_for_select += options_from_collection_for_select(eval("group.#{group_method}"), option_key_method, option_value_method, selected_key)
           options_for_select += '</optgroup>'
-        end
+        end.html_safe
       end
 
       # Returns a string of <tt><option></tt> tags, like <tt>options_for_select</tt>, but
diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb
index 19b73aa810..65b5f5ccc1 100644
--- a/actionpack/test/template/form_options_helper_test.rb
+++ b/actionpack/test/template/form_options_helper_test.rb
@@ -177,17 +177,16 @@ class FormOptionsHelperTest < ActionView::TestCase
   end
 
   def test_option_groups_from_collection_for_select
-    @continents = [
-      Continent.new("<Africa>", [Country.new("<sa>", "<South Africa>"), Country.new("so", "Somalia")] ),
-      Continent.new("Europe", [Country.new("dk", "Denmark"), Country.new("ie", "Ireland")] )
-    ]
-
     assert_dom_equal(
       "<optgroup label=\"&lt;Africa&gt;\"><option value=\"&lt;sa&gt;\">&lt;South Africa&gt;</option>\n<option value=\"so\">Somalia</option></optgroup><optgroup label=\"Europe\"><option value=\"dk\" selected=\"selected\">Denmark</option>\n<option value=\"ie\">Ireland</option></optgroup>",
-      option_groups_from_collection_for_select(@continents, "countries", "continent_name", "country_id", "country_name", "dk")
+      option_groups_from_collection_for_select(dummy_continents, "countries", "continent_name", "country_id", "country_name", "dk")
     )
   end
 
+  def test_option_groups_from_collection_for_select_returns_html_safe_string
+    assert option_groups_from_collection_for_select(dummy_continents, "countries", "continent_name", "country_id", "country_name", "dk").html_safe?
+  end
+
   def test_grouped_options_for_select_with_array
     assert_dom_equal(
       "<optgroup label=\"North America\"><option value=\"US\">United States</option>\n<option value=\"Canada\">Canada</option></optgroup><optgroup label=\"Europe\"><option value=\"GB\">Great Britain</option>\n<option value=\"Germany\">Germany</option></optgroup>",
@@ -824,31 +823,21 @@ class FormOptionsHelperTest < ActionView::TestCase
   end
 
   def test_grouped_collection_select
-    @continents = [
-      Continent.new("<Africa>", [Country.new("<sa>", "<South Africa>"), Country.new("so", "Somalia")] ),
-      Continent.new("Europe", [Country.new("dk", "Denmark"), Country.new("ie", "Ireland")] )
-    ]
-
     @post = Post.new
     @post.origin = 'dk'
 
     assert_dom_equal(
       %Q{<select id="post_origin" name="post[origin]"><optgroup label="&lt;Africa&gt;"><option value="&lt;sa&gt;">&lt;South Africa&gt;</option>\n<option value="so">Somalia</option></optgroup><optgroup label="Europe"><option value="dk" selected="selected">Denmark</option>\n<option value="ie">Ireland</option></optgroup></select>},
-      grouped_collection_select("post", "origin", @continents, :countries, :continent_name, :country_id, :country_name)
+      grouped_collection_select("post", "origin", dummy_continents, :countries, :continent_name, :country_id, :country_name)
     )
   end
 
   def test_grouped_collection_select_under_fields_for
-    @continents = [
-      Continent.new("<Africa>", [Country.new("<sa>", "<South Africa>"), Country.new("so", "Somalia")] ),
-      Continent.new("Europe", [Country.new("dk", "Denmark"), Country.new("ie", "Ireland")] )
-    ]
-
     @post = Post.new
     @post.origin = 'dk'
 
     output_buffer = fields_for :post, @post do |f|
-      concat f.grouped_collection_select("origin", @continents, :countries, :continent_name, :country_id, :country_name)
+      concat f.grouped_collection_select("origin", dummy_continents, :countries, :continent_name, :country_id, :country_name)
     end
 
     assert_dom_equal(
@@ -864,4 +853,9 @@ class FormOptionsHelperTest < ActionView::TestCase
         Post.new("Babe went home", "Babe", "To a little house", "shh!"),
         Post.new("Cabe went home", "Cabe", "To a little house", "shh!") ]
     end
+
+    def dummy_continents
+      [ Continent.new("<Africa>", [Country.new("<sa>", "<South Africa>"), Country.new("so", "Somalia")] ),
+       Continent.new("Europe", [Country.new("dk", "Denmark"), Country.new("ie", "Ireland")] ) ]
+    end
 end