Applied changes to stylesheet_link_tag from javascript_include_tag which corrects issues with ordering and duplicates.

3 files changed, 38 insertions, 14 deletions

diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_include_tag.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_include_tag.rb
index fc0cca28b9..52eb43a1cd 100644
--- a/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_include_tag.rb
+++ b/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_include_tag.rb
@@ -71,9 +71,21 @@ module ActionView
             if sources.first == :all
               collect_asset_files(custom_dir, ('**' if recursive), "*.#{extension}")
             else
-              sources.collect do |source|
-                determine_source(source, expansions)
-              end.flatten
+              sources.inject([]) do |list, source|
+                determined_source = determine_source(source, expansions)
+                update_source_list(list, determined_source)
+              end
+            end
+          end
+
+          def update_source_list(list, source)
+            case source
+            when String
+              list.delete(source)
+              list << source
+            when Array
+              updated_sources = source - list
+              list.concat(updated_sources)
             end
           end
 
diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb
index 4781832711..b9126af944 100644
--- a/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb
+++ b/actionpack/lib/action_view/helpers/asset_tag_helpers/javascript_tag_helpers.rb
@@ -42,17 +42,6 @@ module ActionView
             end
           end
 
-          def update_source_list(list, source)
-            case source
-            when String
-              list.delete(source)
-              list << source
-            when Array
-              updated_sources = source - list
-              list.concat(updated_sources)
-            end
-          end
-
           def add_application_js(expanded_sources, sources)
             if sources.include?(:defaults) && File.exist?(File.join(custom_dir, "application.#{extension}"))
               expanded_sources.delete('application')
diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
index 49e47a2aed..a1a6b5f1d0 100644
--- a/actionpack/test/template/asset_tag_helper_test.rb
+++ b/actionpack/test/template/asset_tag_helper_test.rb
@@ -356,6 +356,29 @@ class AssetTagHelperTest < ActionView::TestCase
     assert_dom_equal  %(<link href="/stylesheets/version.1.0.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/bank.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/robber.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/subdir/subdir.css" media="screen" rel="stylesheet" type="text/css" />), stylesheet_link_tag('version.1.0', :robbery, 'subdir/subdir')
   end
 
+  def test_custom_stylesheet_expansions_return_unique_set
+    ENV["RAILS_ASSET_ID"] = ""
+    ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :cities => %w(wellington amsterdam london)
+    assert_dom_equal  %(<link href="/stylesheets/wellington.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/amsterdam.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/london.css" media="screen" rel="stylesheet" type="text/css" />), stylesheet_link_tag(:cities)
+  end
+
+  def test_stylesheet_link_tag_should_not_output_the_same_asset_twice
+    ENV["RAILS_ASSET_ID"] = ""
+    assert_dom_equal  %(<link href="/stylesheets/wellington.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/amsterdam.css" media="screen" rel="stylesheet" type="text/css" />), stylesheet_link_tag('wellington', 'wellington', 'amsterdam')
+  end
+
+  def test_stylesheet_link_tag_should_not_output_the_same_expansion_twice
+    ENV["RAILS_ASSET_ID"] = ""
+    ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :cities => %w(wellington amsterdam london)
+    assert_dom_equal  %(<link href="/stylesheets/wellington.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/amsterdam.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/london.css" media="screen" rel="stylesheet" type="text/css" />), stylesheet_link_tag(:cities, :cities)
+  end
+
+  def test_single_stylesheet_asset_keys_should_take_precedence_over_expansions
+    ENV["RAILS_ASSET_ID"] = ""
+    ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :cities => %w(wellington amsterdam london)
+    assert_dom_equal  %(<link href="/stylesheets/london.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/wellington.css" media="screen" rel="stylesheet" type="text/css" />\n<link href="/stylesheets/amsterdam.css" media="screen" rel="stylesheet" type="text/css" />), stylesheet_link_tag('london', :cities)
+  end
+
   def test_custom_stylesheet_expansions_with_undefined_symbol
     ActionView::Helpers::AssetTagHelper::register_stylesheet_expansion :monkey => nil
     assert_raise(ArgumentError) { stylesheet_link_tag('first', :monkey, 'last') }