FIX: persist secure image width and height if is given (#10994)

`max-width: 50%; max-height: 400px;` is a good fallback, however, if width and height are given and are smaller than fallback -  we should persist that smaller size.
This commit is contained in:
Krzysztof Kotlarek 2020-10-22 13:25:09 +11:00 committed by GitHub
parent 64b0b50ac0
commit b2481adb40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 27 additions and 8 deletions

View File

@ -6,6 +6,8 @@
#
module Email
class Styles
MAX_IMAGE_DIMENSION = 400
@@plugin_callbacks = []
attr_accessor :fragment
@ -220,7 +222,7 @@ module Email
onebox_styles
plugin_styles
style('.post-excerpt img', "max-width: 50%; max-height: 400px;")
style('.post-excerpt img', "max-width: 50%; max-height: #{MAX_IMAGE_DIMENSION}px;")
format_custom
end
@ -257,7 +259,7 @@ module Email
url = attachments[original_filename].url
div.add_next_sibling(
"<img src=\"#{url}\" data-embedded-secure-image=\"true\" style=\"max-width: 50%; max-height: 400px;\" />"
"<img src=\"#{url}\" data-embedded-secure-image=\"true\" style=\"#{calculate_width_and_height_style(div)}\" />"
)
div.remove
end
@ -320,6 +322,16 @@ module Email
end
end
def calculate_width_and_height_style(div)
width = div['data-width']
height = div['data-height']
if width.present? && height.present? && height.to_i < MAX_IMAGE_DIMENSION && width.to_i < MAX_IMAGE_DIMENSION
"width: #{width}px; height: #{height}px;"
else
"max-width: 50%; max-height: #{MAX_IMAGE_DIMENSION}px;"
end
end
def replace_secure_media_urls
# strip again, this can be done at a lower level like in the user
# notification template but that may not catch everything

View File

@ -409,21 +409,25 @@ module PrettyText
if Upload.secure_media_url?(a["href"])
target = %w(video audio).include?(a&.parent&.name) ? a.parent : a
next if target.to_s.include?("stripped-secure-view-media")
target.add_next_sibling secure_media_placeholder(doc, a['href'])
width = a.xpath("//*[@width]").attr("width")&.value
height = a.xpath("//*[@height]").attr("height")&.value
target.add_next_sibling secure_media_placeholder(doc, a['href'], width: width, height: height)
target.remove
end
end
doc.css('img[src]').each do |img|
if Upload.secure_media_url?(img['src'])
img.add_next_sibling secure_media_placeholder(doc, img['src'])
img.add_next_sibling secure_media_placeholder(doc, img['src'], width: img['width'], height: img['height'])
img.remove
end
end
end
def self.secure_media_placeholder(doc, url)
def self.secure_media_placeholder(doc, url, width: nil, height: nil)
data_width = width ? "data-width=#{width}" : ''
data_height = height ? "data-height=#{height}" : ''
<<~HTML
<div class="secure-media-notice" data-stripped-secure-media="#{url}">
<div class="secure-media-notice" data-stripped-secure-media="#{url}" #{data_width} #{data_height}>
#{I18n.t('emails.secure_media_placeholder')} <a class='stripped-secure-view-media' href="#{url}">#{I18n.t("emails.view_redacted_media")}</a>.
</div>
HTML

View File

@ -212,7 +212,7 @@ describe Email::Styles do
fab!(:upload) { Fabricate(:upload, original_filename: 'testimage.png', secure: true, sha1: '123456') }
def strip_and_inline
html = "<a href=\"#{Discourse.base_url}\/secure-media-uploads/original/1X/123456.png\"><img src=\"/secure-media-uploads/original/1X/123456.png\"></a>"
html = "<a href=\"#{Discourse.base_url}\/secure-media-uploads/original/1X/123456.png\"><img src=\"/secure-media-uploads/original/1X/123456.png\" width=\"20\" height=\"30\"></a>"
# strip out the secure media
styler = Email::Styles.new(html)
@ -230,6 +230,7 @@ describe Email::Styles do
strip_and_inline
expect(@frag.to_s).to include("cid:email/test.png")
expect(@frag.css('[data-stripped-secure-media]')).not_to be_present
expect(@frag.children.attr('style').value).to eq("width: 20px; height: 30px;")
end
it "does not inline anything if the upload cannot be found" do

View File

@ -966,12 +966,14 @@ describe PrettyText do
it "replaces secure images with a placeholder, keeping the url in an attribute" do
url = "/secure-media-uploads/original/1X/testimage.png"
html = <<~HTML
<img src=\"#{url}\">
<img src=\"#{url}\" width=\"20\" height=\"20\">
HTML
md = PrettyText.format_for_email(html, post)
expect(md).not_to include('<img')
expect(md).to include("Redacted")
expect(md).to include("data-stripped-secure-media=\"#{url}\"")
expect(md).to include("data-width=\"20\"")
expect(md).to include("data-height=\"20\"")
end
end
end