From 6c64e1e3a33c2e703a9939f075fb57167b75b36a Mon Sep 17 00:00:00 2001 From: Chris Eppstein Date: Mon, 27 Jun 2011 13:54:54 -0700 Subject: Stylesheet link tags should use the request protocol to avoid duplicate download of stylesheets in IE7 and IE8. Conflicts: actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb actionpack/lib/sprockets/helpers/rails_helper.rb --- actionpack/lib/action_view/asset_paths.rb | 37 +++++++++++++++++++--- .../helpers/asset_tag_helpers/asset_include_tag.rb | 4 +-- .../asset_tag_helpers/stylesheet_tag_helpers.rb | 10 ++---- actionpack/lib/sprockets/helpers/rails_helper.rb | 10 +++--- actionpack/test/template/asset_tag_helper_test.rb | 16 +++++----- 5 files changed, 50 insertions(+), 27 deletions(-) diff --git a/actionpack/lib/action_view/asset_paths.rb b/actionpack/lib/action_view/asset_paths.rb index 4974bd5426..a74d45db6f 100644 --- a/actionpack/lib/action_view/asset_paths.rb +++ b/actionpack/lib/action_view/asset_paths.rb @@ -13,14 +13,20 @@ module ActionView # Prefix with /dir/ if lacking a leading +/+. Account for relative URL # roots. Rewrite the asset path for cache-busting asset ids. Include # asset host, if configured, with the correct request protocol. - def compute_public_path(source, dir, ext = nil, include_host = true) + # + # When include_host is true and the asset host does not specify the protocol + # the protocol parameter specifies how the protocol will be added. + # When :relative (default), the protocol will be determined by the client using current protocol + # When :request, the protocol will be the request protocol + # Otherwise, the protocol is used (E.g. :http, :https, etc) + def compute_public_path(source, dir, ext = nil, include_host = true, protocol = :relative) source = source.to_s return source if is_uri?(source) source = rewrite_extension(source, dir, ext) if ext source = rewrite_asset_path(source, dir) source = rewrite_relative_url_root(source, relative_url_root) if has_request? - source = rewrite_host_and_protocol(source) if include_host + source = rewrite_host_and_protocol(source, protocol) if include_host source end @@ -52,12 +58,33 @@ module ActionView controller.respond_to?(:request) end - def rewrite_host_and_protocol(source) + def rewrite_host_and_protocol(source, protocol = :relative) host = compute_asset_host(source) - host = "//#{host}" if host && !is_uri?(host) + if host && !is_uri?(host) + host = "#{compute_protocol(protocol)}#{host}" + end host.nil? ? source : "#{host}#{source}" end + def compute_protocol(protocol) + protocol ||= :relative + case protocol + when :relative + "//" + when :request + unless @controller + invalid_asset_host!("The protocol requested was :request. Consider using :relative instead.") + end + @controller.request.protocol + else + "#{protocol}://" + end + end + + def invalid_asset_host!(help_message) + raise ActionController::RoutingError, "This asset host cannot be computed without a request in scope. #{help_message}" + end + # Pick an asset host for this source. Returns +nil+ if no host is set, # the host if no wildcard is set, the host interpolated with the # numbers 0-3 if it contains %d (the number is the source hash mod 4), @@ -69,7 +96,7 @@ module ActionView args = [source] arity = arity_of(host) if arity > 1 && !has_request? - raise ActionController::RoutingError, "This asset host cannot be computed without a request in scope. Remove the second argument to your asset_host Proc if you do not need the request." + invalid_asset_host!("Remove the second argument to your asset_host Proc if you do not need the request.") end args << current_request if (arity > 1 || arity < 0) && has_request? host.call(*args) 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 16d8f3f893..621f7aaa0a 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 @@ -60,8 +60,8 @@ module ActionView private - def path_to_asset(source) - asset_paths.compute_public_path(source, asset_name.to_s.pluralize, extension) + def path_to_asset(source, protocol = :relative) + asset_paths.compute_public_path(source, asset_name.to_s.pluralize, extension, true, protocol) end def path_to_asset_source(source) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb index e4f11c9bc7..e829588e94 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helpers/stylesheet_tag_helpers.rb @@ -16,7 +16,7 @@ module ActionView end def asset_tag(source, options) - tag("link", { "rel" => "stylesheet", "type" => Mime::CSS, "media" => "screen", "href" => ERB::Util.html_escape(path_to_asset(source)) }.merge(options), false, false) + tag("link", { "rel" => "stylesheet", "type" => Mime::CSS, "media" => "screen", "href" => ERB::Util.html_escape(path_to_asset(source, :request)) }.merge(options), false, false) end def custom_dir @@ -52,7 +52,7 @@ module ActionView # If the +source+ filename has no extension, .css will be appended (except for explicit URIs). # Full paths from the document root will be passed through. # Used internally by +stylesheet_link_tag+ to build the stylesheet path. - # + # # ==== Examples # stylesheet_path "style" # => /stylesheets/style.css # stylesheet_path "dir/style.css" # => /stylesheets/dir/style.css @@ -60,11 +60,7 @@ module ActionView # stylesheet_path "http://www.example.com/css/style" # => http://www.example.com/css/style # stylesheet_path "http://www.example.com/css/style.css" # => http://www.example.com/css/style.css def stylesheet_path(source) - if config.use_sprockets - asset_path(source, 'css') - else - asset_paths.compute_public_path(source, 'stylesheets', 'css') - end + asset_paths.compute_public_path(source, 'stylesheets', 'css', true, :request) end alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route diff --git a/actionpack/lib/sprockets/helpers/rails_helper.rb b/actionpack/lib/sprockets/helpers/rails_helper.rb index ef7cf9e051..3a77db5fd8 100644 --- a/actionpack/lib/sprockets/helpers/rails_helper.rb +++ b/actionpack/lib/sprockets/helpers/rails_helper.rb @@ -50,7 +50,7 @@ module Sprockets 'rel' => "stylesheet", 'type' => "text/css", 'media' => "screen", - 'href' => asset_path(source, 'css', body) + 'href' => asset_path(source, 'css', body, :request) }.merge(options.stringify_keys) tag 'link', tag_options @@ -58,9 +58,9 @@ module Sprockets end.join("\n").html_safe end - def asset_path(source, default_ext = nil, body = false) + def asset_path(source, default_ext = nil, body = false, protocol = :relative) source = source.logical_path if source.respond_to?(:logical_path) - path = asset_paths.compute_public_path(source, 'assets', default_ext) + path = asset_paths.compute_public_path(source, 'assets', default_ext, protocol) body ? "#{path}?body=1" : path end @@ -71,8 +71,8 @@ module Sprockets end class AssetPaths < ::ActionView::AssetPaths #:nodoc: - def compute_public_path(source, dir, ext=nil, include_host=true) - super(source, Rails.application.config.assets.prefix, ext, include_host) + def compute_public_path(source, dir, ext=nil, include_host=true, protocol = :relative) + super(source, Rails.application.config.assets.prefix, ext, include_host, protocol) end # Return the filesystem path for the source diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index a201a6d1e1..cb7ab2572e 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -854,7 +854,7 @@ class AssetTagHelperTest < ActionView::TestCase def test_caching_stylesheet_link_tag_when_caching_on ENV["RAILS_ASSET_ID"] = "" - @controller.config.asset_host = 'http://a0.example.com' + @controller.config.asset_host = 'a0.example.com' config.perform_caching = true assert_dom_equal( @@ -964,7 +964,7 @@ class AssetTagHelperTest < ActionView::TestCase def test_caching_stylesheet_link_tag_when_caching_on_with_proc_asset_host ENV["RAILS_ASSET_ID"] = "" - @controller.config.asset_host = Proc.new { |source| "http://a#{source.length}.example.com" } + @controller.config.asset_host = Proc.new { |source| "a#{source.length}.example.com" } config.perform_caching = true assert_equal '/stylesheets/styles.css'.length, 23 @@ -1091,13 +1091,13 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase end def test_should_compute_proper_path_with_asset_host - @controller.config.asset_host = "http://assets.example.com" + @controller.config.asset_host = "assets.example.com" assert_dom_equal(%(), auto_discovery_link_tag) - assert_dom_equal(%(http://assets.example.com/collaboration/hieraki/javascripts/xmlhr.js), javascript_path("xmlhr")) - assert_dom_equal(%(http://assets.example.com/collaboration/hieraki/stylesheets/style.css), stylesheet_path("style")) - assert_dom_equal(%(http://assets.example.com/collaboration/hieraki/images/xml.png), image_path("xml.png")) - assert_dom_equal(%(Mouse), image_tag("mouse.png", :mouseover => "/images/mouse_over.png")) - assert_dom_equal(%(Mouse2), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png"))) + assert_dom_equal(%(//assets.example.com/collaboration/hieraki/javascripts/xmlhr.js), javascript_path("xmlhr")) + assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/stylesheets/style.css), stylesheet_path("style")) + assert_dom_equal(%(//assets.example.com/collaboration/hieraki/images/xml.png), image_path("xml.png")) + assert_dom_equal(%(Mouse), image_tag("mouse.png", :mouseover => "/images/mouse_over.png")) + assert_dom_equal(%(Mouse2), image_tag("mouse2.png", :mouseover => image_path("mouse_over2.png"))) end def test_should_ignore_asset_host_on_complete_url -- cgit v1.2.3