From 9a74c7b99bf0ac901f86bb38372a68e157cf9c73 Mon Sep 17 00:00:00 2001 From: Cameron Cundiff Date: Wed, 16 Aug 2017 14:34:02 -0400 Subject: [PATCH] Do not generate default alt text in image tags - Auto-generating content from the filename of an image is not suitable alternative text; alt text that isn't fully considered can be distracting and fatiguing for screen readers users (blind, low vision, dyslexic people). - Setting a filename fallback short circuits screen reader default behavior and configuration for blank descriptions. - Setting poor defaults also creates false negatives for accessibility linting and testing software, that makes it harder to improve application accessibility. *** - After this change, if authors leave images without alt text, screen readers will fallback to default behavior for missing alt text. - Also with this change, Automated linting and testing tools will correctly generate warnings. [Fixes #30096] --- actionmailer/test/asset_host_test.rb | 4 +-- actionmailer/test/base_test.rb | 4 +-- actionmailer/test/url_test.rb | 2 +- actionview/CHANGELOG.md | 6 ++++ .../action_view/helpers/asset_tag_helper.rb | 22 ++++++------- .../test/template/asset_tag_helper_test.rb | 32 +++++++++---------- activestorage/test/template/image_tag_test.rb | 6 ++-- guides/source/action_view_overview.md | 4 +-- guides/source/api_documentation_guidelines.md | 2 +- 9 files changed, 42 insertions(+), 40 deletions(-) diff --git a/actionmailer/test/asset_host_test.rb b/actionmailer/test/asset_host_test.rb index 2a14248488ab7..9cd8cae88cd6f 100644 --- a/actionmailer/test/asset_host_test.rb +++ b/actionmailer/test/asset_host_test.rb @@ -24,7 +24,7 @@ def teardown def test_asset_host_as_string mail = AssetHostMailer.email_with_asset - assert_dom_equal 'Somelogo', mail.body.to_s.strip + assert_dom_equal '', mail.body.to_s.strip end def test_asset_host_as_one_argument_proc @@ -34,6 +34,6 @@ def test_asset_host_as_one_argument_proc end } mail = AssetHostMailer.email_with_asset - assert_dom_equal 'Somelogo', mail.body.to_s.strip + assert_dom_equal '', mail.body.to_s.strip end end diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index d02932949755e..0da969c3498e2 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -578,7 +578,7 @@ def welcome mail = AssetMailer.welcome - assert_dom_equal(%{Dummy}, mail.body.to_s.strip) + assert_dom_equal(%{}, mail.body.to_s.strip) end test "assets tags should use a Mailer's asset_host settings when available" do @@ -592,7 +592,7 @@ def welcome mail = TempAssetMailer.welcome - assert_dom_equal(%{Dummy}, mail.body.to_s.strip) + assert_dom_equal(%{}, mail.body.to_s.strip) end test "the view is not rendered when mail was never called" do diff --git a/actionmailer/test/url_test.rb b/actionmailer/test/url_test.rb index 0bd3371878645..82035689ad53a 100644 --- a/actionmailer/test/url_test.rb +++ b/actionmailer/test/url_test.rb @@ -121,7 +121,7 @@ def test_signed_up_with_url expected = new_mail expected.to = @recipient expected.subject = "[Signed up] Welcome #{@recipient}" - expected.body = "Hello there,\n\nMr. #{@recipient}. Please see our greeting at http://example.com/welcome/greeting http://www.basecamphq.com/welcome\n\n\"Somelogo\"" + expected.body = "Hello there,\n\nMr. #{@recipient}. Please see our greeting at http://example.com/welcome/greeting http://www.basecamphq.com/welcome\n\n" expected.from = "system@loudthinking.com" expected.date = Time.local(2004, 12, 12) expected.content_type = "text/html" diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 5bc93fcb5b20d..b22686d1d88da 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,9 @@ +* Remove default `alt` text generation. + + Fixes #30096 + + *Cameron Cundiff* + * Add `srcset` option to `image_tag` helper. *Roberto Miranda* diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb index 82b0fcbf2ec4e..4557c76dfebf8 100644 --- a/actionview/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb @@ -13,7 +13,7 @@ module Helpers #:nodoc: # the assets exist before linking to them: # # image_tag("rails.png") - # # => Rails + # # => # stylesheet_link_tag("application") # # => module AssetTagHelper @@ -207,8 +207,6 @@ def favicon_link_tag(source = "favicon.ico", options = {}) # You can add HTML attributes using the +options+. The +options+ supports # additional keys for convenience and conformance: # - # * :alt - If no alt text is given, the file name part of the - # +source+ is used (capitalized and without the extension) # * :size - Supplied as "{Width}x{Height}" or "{Number}", so "30x45" becomes # width="30" and height="45", and "50" becomes width="50" and height="50". # :size will be ignored if the value is not in the correct format. @@ -220,17 +218,17 @@ def favicon_link_tag(source = "favicon.ico", options = {}) # Assets (images that are part of your app): # # image_tag("icon") - # # => Icon + # # => # image_tag("icon.png") - # # => Icon + # # => # image_tag("icon.png", size: "16x10", alt: "Edit Entry") # # => Edit Entry # image_tag("/icons/icon.gif", size: "16") - # # => Icon + # # => # image_tag("/icons/icon.gif", height: '32', width: '32') - # # => Icon + # # => # image_tag("/icons/icon.gif", class: "menu_icon") - # # => Icon + # # => # image_tag("/icons/icon.gif", data: { title: 'Rails Application' }) # # => # image_tag("icon.png", srcset: { "icon_2x.png" => "2x", "icon_4x.png" => "4x" }) @@ -251,11 +249,7 @@ def image_tag(source, options = {}) check_for_image_tag_errors(options) skip_pipeline = options.delete(:skip_pipeline) - src = options[:src] = resolve_image_source(source, skip_pipeline) - - unless src.start_with?("cid:") || src.start_with?("data:") || src.blank? - options[:alt] = options.fetch(:alt) { image_alt(src) } - end + options[:src] = resolve_image_source(source, skip_pipeline) if options[:srcset] && !options[:srcset].is_a?(String) options[:srcset] = options[:srcset].map do |src_path, size| @@ -286,6 +280,8 @@ def image_tag(source, options = {}) # image_alt('underscored_file_name.png') # # => Underscored file name def image_alt(src) + ActiveSupport::Deprecation.warn("image_alt is deprecated and will be removed from Rails 6.0. You must explicitly set alt text on images.") + File.basename(src, ".*".freeze).sub(/-[[:xdigit:]]{32,64}\z/, "".freeze).tr("-_".freeze, " ".freeze).capitalize end diff --git a/actionview/test/template/asset_tag_helper_test.rb b/actionview/test/template/asset_tag_helper_test.rb index 5b8e50cc5ece9..d5d4925f5449d 100644 --- a/actionview/test/template/asset_tag_helper_test.rb +++ b/actionview/test/template/asset_tag_helper_test.rb @@ -183,26 +183,26 @@ def url_for(*args) } ImageLinkToTag = { - %(image_tag("xml.png")) => %(Xml), + %(image_tag("xml.png")) => %(), %(image_tag("rss.gif", :alt => "rss syndication")) => %(rss syndication), - %(image_tag("gold.png", :size => "20")) => %(Gold), - %(image_tag("gold.png", :size => 20)) => %(Gold), - %(image_tag("gold.png", :size => "45x70")) => %(Gold), - %(image_tag("gold.png", "size" => "45x70")) => %(Gold), - %(image_tag("error.png", "size" => "45 x 70")) => %(Error), - %(image_tag("error.png", "size" => "x")) => %(Error), - %(image_tag("google.com.png")) => %(Google.com), - %(image_tag("slash..png")) => %(Slash.), - %(image_tag(".pdf.png")) => %(.pdf), - %(image_tag("http://www.rubyonrails.com/images/rails.png")) => %(Rails), - %(image_tag("//www.rubyonrails.com/images/rails.png")) => %(Rails), + %(image_tag("gold.png", :size => "20")) => %(), + %(image_tag("gold.png", :size => 20)) => %(), + %(image_tag("gold.png", :size => "45x70")) => %(), + %(image_tag("gold.png", "size" => "45x70")) => %(), + %(image_tag("error.png", "size" => "45 x 70")) => %(), + %(image_tag("error.png", "size" => "x")) => %(), + %(image_tag("google.com.png")) => %(), + %(image_tag("slash..png")) => %(), + %(image_tag(".pdf.png")) => %(), + %(image_tag("http://www.rubyonrails.com/images/rails.png")) => %(), + %(image_tag("//www.rubyonrails.com/images/rails.png")) => %(), %(image_tag("mouse.png", :alt => nil)) => %(), %(image_tag("data:image/gif;base64,R0lGODlhAQABAID/AMDAwAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==", :alt => nil)) => %(), %(image_tag("")) => %(), - %(image_tag("gold.png", data: { title: 'Rails Application' })) => %(Gold), - %(image_tag("rss.gif", srcset: "/assets/pic_640.jpg 640w, /assets/pic_1024.jpg 1024w")) => %(Rss), - %(image_tag("rss.gif", srcset: { "pic_640.jpg" => "640w", "pic_1024.jpg" => "1024w" })) => %(Rss), - %(image_tag("rss.gif", srcset: [["pic_640.jpg", "640w"], ["pic_1024.jpg", "1024w"]])) => %(Rss) + %(image_tag("gold.png", data: { title: 'Rails Application' })) => %(), + %(image_tag("rss.gif", srcset: "/assets/pic_640.jpg 640w, /assets/pic_1024.jpg 1024w")) => %(), + %(image_tag("rss.gif", srcset: { "pic_640.jpg" => "640w", "pic_1024.jpg" => "1024w" })) => %(), + %(image_tag("rss.gif", srcset: [["pic_640.jpg", "640w"], ["pic_1024.jpg", "1024w"]])) => %() } FaviconLinkToTag = { diff --git a/activestorage/test/template/image_tag_test.rb b/activestorage/test/template/image_tag_test.rb index 46dd97b3e94fe..dedc58452ea1b 100644 --- a/activestorage/test/template/image_tag_test.rb +++ b/activestorage/test/template/image_tag_test.rb @@ -11,17 +11,17 @@ class ActiveStorage::ImageTagTest < ActionView::TestCase end test "blob" do - assert_dom_equal %(Racecar), image_tag(@blob) + assert_dom_equal %(), image_tag(@blob) end test "variant" do variant = @blob.variant(resize: "100x100") - assert_dom_equal %(Racecar), image_tag(variant) + assert_dom_equal %(), image_tag(variant) end test "attachment" do attachment = ActiveStorage::Attachment.new(blob: @blob) - assert_dom_equal %(Racecar), image_tag(attachment) + assert_dom_equal %(), image_tag(attachment) end test "error when attachment's empty" do diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index e5f4e0ec303cc..ea72567c03e2e 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -414,7 +414,7 @@ By default, Rails links to these assets on the current host in the public folder ```ruby config.action_controller.asset_host = "assets.example.com" -image_tag("rails.png") # => Rails +image_tag("rails.png") # => ``` #### auto_discovery_link_tag @@ -453,7 +453,7 @@ image_url("edit.png") # => http://www.example.com/assets/edit.png Returns an HTML image tag for the source. The source can be a full path or a file that exists in your `app/assets/images` directory. ```ruby -image_tag("icon.png") # => Icon +image_tag("icon.png") # => ``` #### javascript_include_tag diff --git a/guides/source/api_documentation_guidelines.md b/guides/source/api_documentation_guidelines.md index c3c7367304d0a..748e2c95b3d05 100644 --- a/guides/source/api_documentation_guidelines.md +++ b/guides/source/api_documentation_guidelines.md @@ -350,7 +350,7 @@ into account, one such example is ```ruby # image_tag("icon.png") -# # => Icon +# # => ``` Although the default behavior for `#image_tag` is to always return