From 1363bb8f7215fadb65e9296217be2ae96e82dd7e Mon Sep 17 00:00:00 2001
From: Josh Kalderimis <josh.kalderimis@gmail.com>
Date: Sun, 13 Feb 2011 05:00:30 +0800
Subject: This corrects two issues with javascript_include_tag, the order at
 which they are expanded, and removing duplicates.

When individual js assets are specified, they will override the order of the same asset specified in an expansion.

[#5938 state:resolved]
---
 .../asset_tag_helpers/javascript_tag_helpers.rb    | 27 ++++++++++++++++++----
 actionpack/test/template/asset_tag_helper_test.rb  | 25 +++++++++++++++++++-
 2 files changed, 47 insertions(+), 5 deletions(-)

(limited to 'actionpack')

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 c95808a219..4781832711 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
@@ -33,13 +33,32 @@ module ActionView
               all_asset_files = (collect_asset_files(custom_dir, ('**' if recursive), "*.#{extension}") - ['application']) << 'application'
               ((determine_source(:defaults, expansions).dup & all_asset_files) + all_asset_files).uniq
             else
-              expanded_sources = sources.collect do |source|
-                determine_source(source, expansions)
-              end.flatten
-              expanded_sources << "application" if sources.include?(:defaults) && File.exist?(File.join(custom_dir, "application.#{extension}"))
+              expanded_sources = sources.inject([]) do |list, source|
+                determined_source = determine_source(source, expansions)
+                update_source_list(list, determined_source)
+              end
+              add_application_js(expanded_sources, sources)
               expanded_sources
             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')
+              expanded_sources << "application"
+            end
+          end
       end
 
 
diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
index 4b4e13e1a7..49e47a2aed 100644
--- a/actionpack/test/template/asset_tag_helper_test.rb
+++ b/actionpack/test/template/asset_tag_helper_test.rb
@@ -95,6 +95,7 @@ class AssetTagHelperTest < ActionView::TestCase
     %(javascript_include_tag(:all)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>),
     %(javascript_include_tag(:all, :recursive => true)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/subdir/subdir.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>),
     %(javascript_include_tag(:defaults, "bank")) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>),
+    %(javascript_include_tag(:defaults, "application")) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>),
     %(javascript_include_tag("bank", :defaults)) => %(<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>),
 
     %(javascript_include_tag("http://example.com/all")) => %(<script src="http://example.com/all" type="text/javascript"></script>),
@@ -265,10 +266,32 @@ class AssetTagHelperTest < ActionView::TestCase
     assert_dom_equal  %(<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>), javascript_include_tag('controls', :robbery, 'effects')
   end
 
+  def test_custom_javascript_expansions_return_unique_set
+    ENV["RAILS_ASSET_ID"] = ""
+    ActionView::Helpers::AssetTagHelper::register_javascript_expansion :defaults => %w(prototype effects dragdrop controls rails application)
+    assert_dom_equal  %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag(:defaults)
+  end
+
   def test_custom_javascript_expansions_and_defaults_puts_application_js_at_the_end
     ENV["RAILS_ASSET_ID"] = ""
     ActionView::Helpers::AssetTagHelper::register_javascript_expansion :robbery => ["bank", "robber"]
-    assert_dom_equal  %(<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag('controls',:defaults, :robbery, 'effects')
+    assert_dom_equal  %(<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag('controls',:defaults, :robbery, 'effects')
+  end
+
+  def test_javascript_include_tag_should_not_output_the_same_asset_twice
+    ENV["RAILS_ASSET_ID"] = ""
+    assert_dom_equal  %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag('prototype', 'effects', :defaults)
+  end
+
+  def test_javascript_include_tag_should_not_output_the_same_expansion_twice
+    ENV["RAILS_ASSET_ID"] = ""
+    assert_dom_equal  %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag(:defaults, :defaults)
+  end
+
+  def test_single_javascript_asset_keys_should_take_precedence_over_expansions
+    ENV["RAILS_ASSET_ID"] = ""
+    assert_dom_equal  %(<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag('controls', :defaults, 'effects')
+    assert_dom_equal  %(<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag('controls', 'effects', :defaults)
   end
 
   def test_registering_javascript_expansions_merges_with_existing_expansions
-- 
cgit v1.2.3