Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not generate default alt text for images #30213

Merged
merged 1 commit into from Aug 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions actionmailer/test/asset_host_test.rb
Expand Up @@ -24,7 +24,7 @@ def teardown

def test_asset_host_as_string
mail = AssetHostMailer.email_with_asset
assert_dom_equal '<img alt="Somelogo" src="http://www.example.com/images/somelogo.png" />', mail.body.to_s.strip
assert_dom_equal '<img src="http://www.example.com/images/somelogo.png" />', mail.body.to_s.strip
end

def test_asset_host_as_one_argument_proc
Expand All @@ -34,6 +34,6 @@ def test_asset_host_as_one_argument_proc
end
}
mail = AssetHostMailer.email_with_asset
assert_dom_equal '<img alt="Somelogo" src="http://images.example.com/images/somelogo.png" />', mail.body.to_s.strip
assert_dom_equal '<img src="http://images.example.com/images/somelogo.png" />', mail.body.to_s.strip
end
end
4 changes: 2 additions & 2 deletions actionmailer/test/base_test.rb
Expand Up @@ -578,7 +578,7 @@ def welcome

mail = AssetMailer.welcome

assert_dom_equal(%{<img alt="Dummy" src="http://global.com/images/dummy.png" />}, mail.body.to_s.strip)
assert_dom_equal(%{<img src="http://global.com/images/dummy.png" />}, mail.body.to_s.strip)
end

test "assets tags should use a Mailer's asset_host settings when available" do
Expand All @@ -592,7 +592,7 @@ def welcome

mail = TempAssetMailer.welcome

assert_dom_equal(%{<img alt="Dummy" src="http://local.com/images/dummy.png" />}, mail.body.to_s.strip)
assert_dom_equal(%{<img src="http://local.com/images/dummy.png" />}, mail.body.to_s.strip)
end

test "the view is not rendered when mail was never called" do
Expand Down
2 changes: 1 addition & 1 deletion actionmailer/test/url_test.rb
Expand Up @@ -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<img alt=\"Somelogo\" src=\"/images/somelogo.png\" />"
expected.body = "Hello there,\n\nMr. #{@recipient}. Please see our greeting at http://example.com/welcome/greeting http://www.basecamphq.com/welcome\n\n<img src=\"/images/somelogo.png\" />"
expected.from = "system@loudthinking.com"
expected.date = Time.local(2004, 12, 12)
expected.content_type = "text/html"
Expand Down
6 changes: 6 additions & 0 deletions 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*
Expand Down
22 changes: 9 additions & 13 deletions actionview/lib/action_view/helpers/asset_tag_helper.rb
Expand Up @@ -13,7 +13,7 @@ module Helpers #:nodoc:
# the assets exist before linking to them:
#
# image_tag("rails.png")
# # => <img alt="Rails" src="/assets/rails.png" />
# # => <img src="/assets/rails.png" />
# stylesheet_link_tag("application")
# # => <link href="/assets/application.css?body=1" media="screen" rel="stylesheet" />
module AssetTagHelper
Expand Down Expand Up @@ -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:
#
# * <tt>:alt</tt> - If no alt text is given, the file name part of the
# +source+ is used (capitalized and without the extension)
# * <tt>:size</tt> - Supplied as "{Width}x{Height}" or "{Number}", so "30x45" becomes
# width="30" and height="45", and "50" becomes width="50" and height="50".
# <tt>:size</tt> will be ignored if the value is not in the correct format.
Expand All @@ -220,17 +218,17 @@ def favicon_link_tag(source = "favicon.ico", options = {})
# Assets (images that are part of your app):
#
# image_tag("icon")
# # => <img alt="Icon" src="/assets/icon" />
# # => <img src="/assets/icon" />
# image_tag("icon.png")
# # => <img alt="Icon" src="/assets/icon.png" />
# # => <img src="/assets/icon.png" />
# image_tag("icon.png", size: "16x10", alt: "Edit Entry")
# # => <img src="/assets/icon.png" width="16" height="10" alt="Edit Entry" />
# image_tag("/icons/icon.gif", size: "16")
# # => <img src="/icons/icon.gif" width="16" height="16" alt="Icon" />
# # => <img src="/icons/icon.gif" width="16" height="16" />
# image_tag("/icons/icon.gif", height: '32', width: '32')
# # => <img alt="Icon" height="32" src="/icons/icon.gif" width="32" />
# # => <img height="32" src="/icons/icon.gif" width="32" />
# image_tag("/icons/icon.gif", class: "menu_icon")
# # => <img alt="Icon" class="menu_icon" src="/icons/icon.gif" />
# # => <img class="menu_icon" src="/icons/icon.gif" />
# image_tag("/icons/icon.gif", data: { title: 'Rails Application' })
# # => <img data-title="Rails Application" src="/icons/icon.gif" />
# image_tag("icon.png", srcset: { "icon_2x.png" => "2x", "icon_4x.png" => "4x" })
Expand All @@ -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|
Expand Down Expand Up @@ -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

Expand Down
32 changes: 16 additions & 16 deletions actionview/test/template/asset_tag_helper_test.rb
Expand Up @@ -183,26 +183,26 @@ def url_for(*args)
}

ImageLinkToTag = {
%(image_tag("xml.png")) => %(<img alt="Xml" src="/images/xml.png" />),
%(image_tag("xml.png")) => %(<img src="/images/xml.png" />),
%(image_tag("rss.gif", :alt => "rss syndication")) => %(<img alt="rss syndication" src="/images/rss.gif" />),
%(image_tag("gold.png", :size => "20")) => %(<img alt="Gold" height="20" src="/images/gold.png" width="20" />),
%(image_tag("gold.png", :size => 20)) => %(<img alt="Gold" height="20" src="/images/gold.png" width="20" />),
%(image_tag("gold.png", :size => "45x70")) => %(<img alt="Gold" height="70" src="/images/gold.png" width="45" />),
%(image_tag("gold.png", "size" => "45x70")) => %(<img alt="Gold" height="70" src="/images/gold.png" width="45" />),
%(image_tag("error.png", "size" => "45 x 70")) => %(<img alt="Error" src="/images/error.png" />),
%(image_tag("error.png", "size" => "x")) => %(<img alt="Error" src="/images/error.png" />),
%(image_tag("google.com.png")) => %(<img alt="Google.com" src="/images/google.com.png" />),
%(image_tag("slash..png")) => %(<img alt="Slash." src="/images/slash..png" />),
%(image_tag(".pdf.png")) => %(<img alt=".pdf" src="/images/.pdf.png" />),
%(image_tag("http://www.rubyonrails.com/images/rails.png")) => %(<img alt="Rails" src="http://www.rubyonrails.com/images/rails.png" />),
%(image_tag("//www.rubyonrails.com/images/rails.png")) => %(<img alt="Rails" src="//www.rubyonrails.com/images/rails.png" />),
%(image_tag("gold.png", :size => "20")) => %(<img height="20" src="/images/gold.png" width="20" />),
%(image_tag("gold.png", :size => 20)) => %(<img height="20" src="/images/gold.png" width="20" />),
%(image_tag("gold.png", :size => "45x70")) => %(<img height="70" src="/images/gold.png" width="45" />),
%(image_tag("gold.png", "size" => "45x70")) => %(<img height="70" src="/images/gold.png" width="45" />),
%(image_tag("error.png", "size" => "45 x 70")) => %(<img src="/images/error.png" />),
%(image_tag("error.png", "size" => "x")) => %(<img src="/images/error.png" />),
%(image_tag("google.com.png")) => %(<img src="/images/google.com.png" />),
%(image_tag("slash..png")) => %(<img src="/images/slash..png" />),
%(image_tag(".pdf.png")) => %(<img src="/images/.pdf.png" />),
%(image_tag("http://www.rubyonrails.com/images/rails.png")) => %(<img src="http://www.rubyonrails.com/images/rails.png" />),
%(image_tag("//www.rubyonrails.com/images/rails.png")) => %(<img src="//www.rubyonrails.com/images/rails.png" />),
%(image_tag("mouse.png", :alt => nil)) => %(<img src="/images/mouse.png" />),
%(image_tag("data:image/gif;base64,R0lGODlhAQABAID/AMDAwAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==", :alt => nil)) => %(<img src="data:image/gif;base64,R0lGODlhAQABAID/AMDAwAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==" />),
%(image_tag("")) => %(<img src="" />),
%(image_tag("gold.png", data: { title: 'Rails Application' })) => %(<img data-title="Rails Application" src="/images/gold.png" alt="Gold" />),
%(image_tag("rss.gif", srcset: "/assets/pic_640.jpg 640w, /assets/pic_1024.jpg 1024w")) => %(<img srcset="/assets/pic_640.jpg 640w, /assets/pic_1024.jpg 1024w" src="/images/rss.gif" alt="Rss" />),
%(image_tag("rss.gif", srcset: { "pic_640.jpg" => "640w", "pic_1024.jpg" => "1024w" })) => %(<img srcset="/images/pic_640.jpg 640w, /images/pic_1024.jpg 1024w" src="/images/rss.gif" alt="Rss" />),
%(image_tag("rss.gif", srcset: [["pic_640.jpg", "640w"], ["pic_1024.jpg", "1024w"]])) => %(<img srcset="/images/pic_640.jpg 640w, /images/pic_1024.jpg 1024w" src="/images/rss.gif" alt="Rss" />)
%(image_tag("gold.png", data: { title: 'Rails Application' })) => %(<img data-title="Rails Application" src="/images/gold.png" />),
%(image_tag("rss.gif", srcset: "/assets/pic_640.jpg 640w, /assets/pic_1024.jpg 1024w")) => %(<img srcset="/assets/pic_640.jpg 640w, /assets/pic_1024.jpg 1024w" src="/images/rss.gif" />),
%(image_tag("rss.gif", srcset: { "pic_640.jpg" => "640w", "pic_1024.jpg" => "1024w" })) => %(<img srcset="/images/pic_640.jpg 640w, /images/pic_1024.jpg 1024w" src="/images/rss.gif" />),
%(image_tag("rss.gif", srcset: [["pic_640.jpg", "640w"], ["pic_1024.jpg", "1024w"]])) => %(<img srcset="/images/pic_640.jpg 640w, /images/pic_1024.jpg 1024w" src="/images/rss.gif" />)
}

FaviconLinkToTag = {
Expand Down
6 changes: 3 additions & 3 deletions activestorage/test/template/image_tag_test.rb
Expand Up @@ -11,17 +11,17 @@ class ActiveStorage::ImageTagTest < ActionView::TestCase
end

test "blob" do
assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url @blob}" />), image_tag(@blob)
assert_dom_equal %(<img src="#{polymorphic_url @blob}" />), image_tag(@blob)
end

test "variant" do
variant = @blob.variant(resize: "100x100")
assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url variant}" />), image_tag(variant)
assert_dom_equal %(<img src="#{polymorphic_url variant}" />), image_tag(variant)
end

test "attachment" do
attachment = ActiveStorage::Attachment.new(blob: @blob)
assert_dom_equal %(<img alt="Racecar" src="#{polymorphic_url attachment}" />), image_tag(attachment)
assert_dom_equal %(<img src="#{polymorphic_url attachment}" />), image_tag(attachment)
end

test "error when attachment's empty" do
Expand Down
4 changes: 2 additions & 2 deletions guides/source/action_view_overview.md
Expand Up @@ -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") # => <img src="http://assets.example.com/images/rails.png" alt="Rails" />
image_tag("rails.png") # => <img src="http://assets.example.com/images/rails.png" />
```

#### auto_discovery_link_tag
Expand Down Expand Up @@ -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") # => <img src="/assets/icon.png" alt="Icon" />
image_tag("icon.png") # => <img src="/assets/icon.png" />
```

#### javascript_include_tag
Expand Down
2 changes: 1 addition & 1 deletion guides/source/api_documentation_guidelines.md
Expand Up @@ -350,7 +350,7 @@ into account, one such example is

```ruby
# image_tag("icon.png")
# # => <img alt="Icon" src="/assets/icon.png" />
# # => <img src="/assets/icon.png" />
```

Although the default behavior for `#image_tag` is to always return
Expand Down