Remove legacy media=screen default from stylesheet_link_tag.

If the media attribute is omitted, the default for web browsers is "all", meaning that by default links apply to all media. Before: ```ruby > stylesheet_link_tag "style" => <link href="/assets/style.css" media="screen" rel="stylesheet" /> ``` After: ```ruby > stylesheet_link_tag "style" => <link href="/assets/style.css" rel="stylesheet" /> ``` The current behavior is not going to change for existing applications. For newly built applications, the media attribute is not going to be added by default. Which can be configured using the following: ``` Rails.application.config.action_view.stylesheet_media_default = false ```
2021-01-22 19:54:40 -03:00 · 2021-01-22 19:54:40 -03:00 · 1280620767
commit 1280620767
parent 76c834bcb6
12 changed files with 75 additions and 36 deletions
--- a/actionview/lib/action_view/helpers/asset_tag_helper.rb
+++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb
@ -16,7 +16,7 @@ module Helpers #:nodoc:
    #   image_tag("rails.png")
    #   # => <img src="/assets/rails.png" />
    #   stylesheet_link_tag("application")
-    #   # => <link href="/assets/application.css?body=1" media="screen" rel="stylesheet" />
+    #   # => <link href="/assets/application.css?body=1" rel="stylesheet" />
    module AssetTagHelper
      include AssetUrlHelper
      include TagHelper
@ -24,6 +24,7 @@ module AssetTagHelper
      mattr_accessor :image_loading
      mattr_accessor :image_decoding
      mattr_accessor :preload_links_header
+      mattr_accessor :stylesheet_media_default

      # Returns an HTML script tag for each of the +sources+ provided.
      #
@ -123,21 +124,18 @@ def javascript_include_tag(*sources)
      # Returns a stylesheet link tag for the sources specified as arguments. If
      # you don't specify an extension, <tt>.css</tt> will be appended automatically.
      # You can modify the link attributes by passing a hash as the last argument.
-      # For historical reasons, the 'media' attribute will always be present and defaults
-      # to "screen", so you must explicitly set it to "all" for the stylesheet(s) to
-      # apply to all media types.
      #
      # If the server supports Early Hints header links for these assets will be
      # automatically pushed.
      #
      #   stylesheet_link_tag "style"
-      #   # => <link href="/assets/style.css" media="screen" rel="stylesheet" />
+      #   # => <link href="/assets/style.css" rel="stylesheet" />
      #
      #   stylesheet_link_tag "style.css"
-      #   # => <link href="/assets/style.css" media="screen" rel="stylesheet" />
+      #   # => <link href="/assets/style.css" rel="stylesheet" />
      #
      #   stylesheet_link_tag "http://www.example.com/style.css"
-      #   # => <link href="http://www.example.com/style.css" media="screen" rel="stylesheet" />
+      #   # => <link href="http://www.example.com/style.css" rel="stylesheet" />
      #
      #   stylesheet_link_tag "style", media: "all"
      #   # => <link href="/assets/style.css" media="all" rel="stylesheet" />
@ -146,8 +144,8 @@ def javascript_include_tag(*sources)
      #   # => <link href="/assets/style.css" media="print" rel="stylesheet" />
      #
      #   stylesheet_link_tag "random.styles", "/css/stylish"
-      #   # => <link href="/assets/random.styles" media="screen" rel="stylesheet" />
-      #   #    <link href="/css/stylish.css" media="screen" rel="stylesheet" />
+      #   # => <link href="/assets/random.styles" rel="stylesheet" />
+      #   #    <link href="/css/stylish.css" rel="stylesheet" />
      def stylesheet_link_tag(*sources)
        options = sources.extract_options!.stringify_keys
        path_options = options.extract!("protocol", "host", "skip_pipeline").symbolize_keys
@ -168,10 +166,14 @@ def stylesheet_link_tag(*sources)
          end
          tag_options = {
            "rel" => "stylesheet",
-            "media" => "screen",
            "crossorigin" => crossorigin,
            "href" => href
          }.merge!(options)
+
+          if stylesheet_media_default && tag_options[:media].blank?
+            tag_options[:media] = "screen"
+          end
+
          tag(:link, tag_options)
        }.join("\n").html_safe

--- a/actionview/lib/action_view/helpers/asset_url_helper.rb
+++ b/actionview/lib/action_view/helpers/asset_url_helper.rb
@ -31,7 +31,7 @@ module Helpers #:nodoc:
    #   image_tag("rails.png")
    #   # => <img src="http://assets.example.com/assets/rails.png" />
    #   stylesheet_link_tag("application")
-    #   # => <link href="http://assets.example.com/assets/application.css" media="screen" rel="stylesheet" />
+    #   # => <link href="http://assets.example.com/assets/application.css" rel="stylesheet" />
    #
    # Browsers open a limited number of simultaneous connections to a single
    # host. The exact number varies by browser and version. This limit may cause
@ -44,7 +44,7 @@ module Helpers #:nodoc:
    #   image_tag("rails.png")
    #   # => <img src="http://assets0.example.com/assets/rails.png" />
    #   stylesheet_link_tag("application")
-    #   # => <link href="http://assets2.example.com/assets/application.css" media="screen" rel="stylesheet" />
+    #   # => <link href="http://assets2.example.com/assets/application.css" rel="stylesheet" />
    #
    # This may improve the asset loading performance of your application.
    # It is also possible the combination of additional connection overhead
@ -70,7 +70,7 @@ module Helpers #:nodoc:
    #   image_tag("rails.png")
    #   # => <img src="http://assets1.example.com/assets/rails.png" />
    #   stylesheet_link_tag("application")
-    #   # => <link href="http://assets2.example.com/assets/application.css" media="screen" rel="stylesheet" />
+    #   # => <link href="http://assets2.example.com/assets/application.css" rel="stylesheet" />
    #
    # The example above generates "http://assets1.example.com" and
    # "http://assets2.example.com". This option is useful for example if
@ -89,7 +89,7 @@ module Helpers #:nodoc:
    #   image_tag("rails.png")
    #   # => <img src="http://assets.example.com/assets/rails.png" />
    #   stylesheet_link_tag("application")
-    #   # => <link href="http://stylesheets.example.com/assets/application.css" media="screen" rel="stylesheet" />
+    #   # => <link href="http://stylesheets.example.com/assets/application.css" rel="stylesheet" />
    #
    # Alternatively you may ask for a second parameter +request+. That one is
    # particularly useful for serving assets from an SSL-protected page. The
--- a/actionview/lib/action_view/railtie.rb
+++ b/actionview/lib/action_view/railtie.rb
@ -12,6 +12,7 @@ class Railtie < Rails::Engine # :nodoc:
    config.action_view.default_enforce_utf8 = nil
    config.action_view.image_loading = nil
    config.action_view.image_decoding = nil
+    config.action_view.stylesheet_media_default = true

    config.eager_load_namespaces << ActionView

@ -50,6 +51,7 @@ class Railtie < Rails::Engine # :nodoc:
      ActionView::Helpers::AssetTagHelper.image_loading = app.config.action_view.delete(:image_loading)
      ActionView::Helpers::AssetTagHelper.image_decoding = app.config.action_view.delete(:image_decoding)
      ActionView::Helpers::AssetTagHelper.preload_links_header = app.config.action_view.delete(:preload_links_header)
+      ActionView::Helpers::AssetTagHelper.stylesheet_media_default = app.config.action_view.delete(:stylesheet_media_default)
    end

    config.after_initialize do |app|
--- a/actionview/test/template/asset_tag_helper_test.rb
+++ b/actionview/test/template/asset_tag_helper_test.rb
@ -160,16 +160,16 @@ def content_security_policy_nonce
  }

  StyleLinkToTag = {
-    %(stylesheet_link_tag("bank")) => %(<link href="/stylesheets/bank.css" media="screen" rel="stylesheet" />),
-    %(stylesheet_link_tag("bank.css")) => %(<link href="/stylesheets/bank.css" media="screen" rel="stylesheet" />),
-    %(stylesheet_link_tag("/elsewhere/file")) => %(<link href="/elsewhere/file.css" media="screen" rel="stylesheet" />),
-    %(stylesheet_link_tag("subdir/subdir")) => %(<link href="/stylesheets/subdir/subdir.css" media="screen" rel="stylesheet" />),
+    %(stylesheet_link_tag("bank")) => %(<link href="/stylesheets/bank.css" rel="stylesheet" />),
+    %(stylesheet_link_tag("bank.css")) => %(<link href="/stylesheets/bank.css" rel="stylesheet" />),
+    %(stylesheet_link_tag("/elsewhere/file")) => %(<link href="/elsewhere/file.css" rel="stylesheet" />),
+    %(stylesheet_link_tag("subdir/subdir")) => %(<link href="/stylesheets/subdir/subdir.css" rel="stylesheet" />),
    %(stylesheet_link_tag("bank", :media => "all")) => %(<link href="/stylesheets/bank.css" media="all" rel="stylesheet" />),
-    %(stylesheet_link_tag("bank", :host => "assets.example.com")) => %(<link href="http://assets.example.com/stylesheets/bank.css" media="screen" rel="stylesheet" />),
+    %(stylesheet_link_tag("bank", :host => "assets.example.com")) => %(<link href="http://assets.example.com/stylesheets/bank.css" rel="stylesheet" />),

-    %(stylesheet_link_tag("http://www.example.com/styles/style")) => %(<link href="http://www.example.com/styles/style" media="screen" rel="stylesheet" />),
-    %(stylesheet_link_tag("http://www.example.com/styles/style.css")) => %(<link href="http://www.example.com/styles/style.css" media="screen" rel="stylesheet" />),
-    %(stylesheet_link_tag("//www.example.com/styles/style.css")) => %(<link href="//www.example.com/styles/style.css" media="screen" rel="stylesheet" />),
+    %(stylesheet_link_tag("http://www.example.com/styles/style")) => %(<link href="http://www.example.com/styles/style" rel="stylesheet" />),
+    %(stylesheet_link_tag("http://www.example.com/styles/style.css")) => %(<link href="http://www.example.com/styles/style.css" rel="stylesheet" />),
+    %(stylesheet_link_tag("//www.example.com/styles/style.css")) => %(<link href="//www.example.com/styles/style.css" rel="stylesheet" />),
  }

  ImagePathToTag = {
@ -476,7 +476,7 @@ def test_stylesheet_link_tag_with_missing_source
  def test_stylesheet_link_tag_without_request
    @request = nil
    assert_dom_equal(
-      %(<link rel="stylesheet" media="screen" href="/stylesheets/foo.css" />),
+      %(<link rel="stylesheet" href="/stylesheets/foo.css" />),
      stylesheet_link_tag("foo.css")
    )
  end
@ -491,18 +491,28 @@ def test_stylesheet_link_tag_escapes_options
  end

  def test_stylesheet_link_tag_should_not_output_the_same_asset_twice
-    assert_dom_equal %(<link href="/stylesheets/wellington.css" media="screen" rel="stylesheet" />\n<link href="/stylesheets/amsterdam.css" media="screen" rel="stylesheet" />), stylesheet_link_tag("wellington", "wellington", "amsterdam")
+    assert_dom_equal %(<link href="/stylesheets/wellington.css" rel="stylesheet" />\n<link href="/stylesheets/amsterdam.css" rel="stylesheet" />), stylesheet_link_tag("wellington", "wellington", "amsterdam")
  end

  def test_stylesheet_link_tag_with_relative_protocol
    @controller.config.asset_host = "assets.example.com"
-    assert_dom_equal %(<link href="//assets.example.com/stylesheets/wellington.css" media="screen" rel="stylesheet" />), stylesheet_link_tag("wellington", protocol: :relative)
+    assert_dom_equal %(<link href="//assets.example.com/stylesheets/wellington.css" rel="stylesheet" />), stylesheet_link_tag("wellington", protocol: :relative)
  end

  def test_stylesheet_link_tag_with_default_protocol
    @controller.config.asset_host = "assets.example.com"
    @controller.config.default_asset_host_protocol = :relative
-    assert_dom_equal %(<link href="//assets.example.com/stylesheets/wellington.css" media="screen" rel="stylesheet" />), stylesheet_link_tag("wellington")
+    assert_dom_equal %(<link href="//assets.example.com/stylesheets/wellington.css" rel="stylesheet" />), stylesheet_link_tag("wellington")
+  end
+
+  def test_stylesheet_link_tag_with_configured_stylesheet_media_default
+    original_default_media = ActionView::Helpers::AssetTagHelper.stylesheet_media_default
+    ActionView::Helpers::AssetTagHelper.stylesheet_media_default = true
+
+    assert_dom_equal %(<link href="/file.css" media="screen" rel="stylesheet" />), stylesheet_link_tag("/file")
+    assert_dom_equal %(<link href="/file.css" media="all" rel="stylesheet" />), stylesheet_link_tag("/file", media: "all")
+  ensure
+    ActionView::Helpers::AssetTagHelper.stylesheet_media_default = original_default_media
  end

  def test_javascript_include_tag_without_request
@ -852,12 +862,12 @@ def test_should_return_asset_host_with_protocol

  def test_should_ignore_asset_host_on_complete_url
    @controller.config.asset_host = "http://assets.example.com"
-    assert_dom_equal(%(<link href="http://bar.example.com/stylesheets/style.css" media="screen" rel="stylesheet" />), stylesheet_link_tag("http://bar.example.com/stylesheets/style.css"))
+    assert_dom_equal(%(<link href="http://bar.example.com/stylesheets/style.css" rel="stylesheet" />), stylesheet_link_tag("http://bar.example.com/stylesheets/style.css"))
  end

  def test_should_ignore_asset_host_on_scheme_relative_url
    @controller.config.asset_host = "http://assets.example.com"
-    assert_dom_equal(%(<link href="//bar.example.com/stylesheets/style.css" media="screen" rel="stylesheet" />), stylesheet_link_tag("//bar.example.com/stylesheets/style.css"))
+    assert_dom_equal(%(<link href="//bar.example.com/stylesheets/style.css" rel="stylesheet" />), stylesheet_link_tag("//bar.example.com/stylesheets/style.css"))
  end

  def test_should_wildcard_asset_host
@ -896,7 +906,7 @@ class AssetTagHelperWithoutRequestTest < ActionView::TestCase

  def test_stylesheet_link_tag_without_request
    assert_dom_equal(
-      %(<link rel="stylesheet" media="screen" href="/stylesheets/foo.css" />),
+      %(<link rel="stylesheet" href="/stylesheets/foo.css" />),
      stylesheet_link_tag("foo.css")
    )
  end
--- a/guides/source/action_view_helpers.md
+++ b/guides/source/action_view_helpers.md
@ -103,7 +103,7 @@ Returns a stylesheet link tag for the sources specified as arguments. If you don

 ```ruby
 stylesheet_link_tag "application"
-# => <link href="/assets/application.css" media="screen" rel="stylesheet" />
+# => <link href="/assets/application.css" rel="stylesheet" />
 ```

 #### stylesheet_path
--- a/guides/source/asset_pipeline.md
+++ b/guides/source/asset_pipeline.md
@ -642,8 +642,7 @@ generates something like this:

 ```html
 <script src="/assets/application-908e25f4bf641868d8683022a5b62f54.js"></script>
-<link href="/assets/application-4dd5b109ee3439da54f5bdfd78a80473.css" media="screen"
-rel="stylesheet" />
+<link href="/assets/application-4dd5b109ee3439da54f5bdfd78a80473.css" rel="stylesheet" />
 ```

 NOTE: with the Asset Pipeline the `:cache` and `:concat` options aren't used
--- a/guides/source/configuring.md
+++ b/guides/source/configuring.md
@ -736,6 +736,8 @@ Defaults to `'signed cookie'`.

 * `config.action_view.button_to_generates_button_tag` determines whether `button_to` will render `<button>` element, regardless of whether or not the content is passed as the first argument or as a block.

+* `config.action_view.stylesheet_media_default` determines whether `stylesheet_link_tag` will render `screen` as the default value for the attribute `media` when it's not provided.
+
 ### Configuring Action Mailbox

 `config.action_mailbox` provides the following configuration options:
@ -1043,6 +1045,7 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla

 #### For '6.2', defaults from previous versions below and:
 - `config.action_view.button_to_generates_button_tag`: `true`
+- `config.action_view.stylesheet_media_default` : `false`
 - `config.active_support.key_generator_hash_digest_class`: `OpenSSL::Digest::SHA256`
 - `config.active_support.hash_digest_class`: `OpenSSL::Digest::SHA256`

--- a/guides/source/layouts_and_rendering.md
+++ b/guides/source/layouts_and_rendering.md
@ -915,7 +915,7 @@ To include `http://example.com/main.css`:
 <%= stylesheet_link_tag "http://example.com/main.css" %>
 ```

-By default, the `stylesheet_link_tag` creates links with `media="screen" rel="stylesheet"`. You can override any of these defaults by specifying an appropriate option (`:media`, `:rel`):
+By default, the `stylesheet_link_tag` creates links with `rel="stylesheet"`. You can override this default by specifying an appropriate option (`:rel`):

 ```erb
 <%= stylesheet_link_tag "main_print", media: "print" %>
--- a/railties/lib/rails/application/configuration.rb
+++ b/railties/lib/rails/application/configuration.rb
@ -204,6 +204,7 @@ def load_defaults(target_version)

          if respond_to?(:action_view)
            action_view.button_to_generates_button_tag = true
+            action_view.stylesheet_media_default = false
          end

          if respond_to?(:active_support)
--- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_2.rb.tt
+++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_2.rb.tt
@ -10,6 +10,9 @@
 # or not the content is passed as the first argument or as a block.
 # Rails.application.config.action_view.button_to_generates_button_tag = true

+# `stylesheet_link_tag` view helper will not render the media attribute by default.
+# Rails.application.config.action_view.stylesheet_media_default = false
+
 # Change the digest class for the key generators to `OpenSSL::Digest::SHA256`.
 # Changing this default means invalidate all encrypted messages generated by
 # your application and, all the encrypted cookies. Only change this after you
--- a/railties/test/application/asset_debugging_test.rb
+++ b/railties/test/application/asset_debugging_test.rb
@ -78,7 +78,7 @@ class ::PostsController < ActionController::Base ; end
        stylesheet_path:        %r{/stylesheets/#{contents}},
        image_tag:              %r{<img src="/images/#{contents}"},
        favicon_link_tag:       %r{<link rel="shortcut icon" type="image/x-icon" href="/images/#{contents}" />},
-        stylesheet_link_tag:    %r{<link rel="stylesheet" media="screen" href="/stylesheets/#{contents}.css" />},
+        stylesheet_link_tag:    %r{<link rel="stylesheet" href="/stylesheets/#{contents}.css" />},
        javascript_include_tag: %r{<script src="/javascripts/#{contents}.js">},
        audio_tag:              %r{<audio src="/audios/#{contents}"></audio>},
        video_tag:              %r{<video src="/videos/#{contents}"></video>},
--- a/railties/test/application/configuration_test.rb
+++ b/railties/test/application/configuration_test.rb
@ -2455,6 +2455,25 @@ class ::DummySerializer < ActiveJob::Serializers::ObjectSerializer; end
      assert_equal false, ActionView::Helpers::AssetTagHelper.preload_links_header
    end

+    test "ActionView::Helpers::AssetTagHelper.stylesheet_media_default is true by default for upgraded apps" do
+      remove_from_config '.*config\.load_defaults.*\n'
+      app "development"
+
+      assert_equal true, ActionView::Helpers::AssetTagHelper.stylesheet_media_default
+    end
+
+    test "ActionView::Helpers::AssetTagHelper.stylesheet_media_default can be configured via config.action_view.stylesheet_media_default" do
+      remove_from_config '.*config\.load_defaults.*\n'
+
+      app_file "config/initializers/new_framework_defaults_6_2.rb", <<-RUBY
+        Rails.application.config.action_view.stylesheet_media_default = false
+      RUBY
+
+      app "development"
+
+      assert_equal false, ActionView::Helpers::AssetTagHelper.stylesheet_media_default
+    end
+
    test "stylesheet_link_tag sets the Link header by default" do
      app_file "app/controllers/pages_controller.rb", <<-RUBY
      class PagesController < ApplicationController
@ -2473,7 +2492,7 @@ def index
      app "development"

      get "/"
-      assert_match %r[<link rel="stylesheet" media="screen" href="/application.css" />], last_response.body
+      assert_match %r[<link rel="stylesheet" href="/application.css" />], last_response.body
      assert_equal "</application.css>; rel=preload; as=style; nopush", last_response.headers["Link"]
    end

@ -2499,7 +2518,7 @@ def index
      app "development"

      get "/"
-      assert_match %r[<link rel="stylesheet" media="screen" href="/application.css" />], last_response.body
+      assert_match %r[<link rel="stylesheet" href="/application.css" />], last_response.body
      assert_nil last_response.headers["Link"]
    end