From 879e4a9e2952f648c84c0c01b5342d37a073204c Mon Sep 17 00:00:00 2001 From: Martin Brennan <mjrbrennan@gmail.com> Date: Mon, 16 Nov 2020 09:58:40 +1000 Subject: [PATCH] FIX: Inline avatar style for onebox when embedding secure images (#11229) When embedding secure images that are inline-avatars for oneboxes we weren't applying the correct sizing/style. --- lib/email/styles.rb | 9 +++-- lib/pretty_text.rb | 20 +++++++---- spec/components/email/styles_spec.rb | 52 ++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 9 deletions(-) diff --git a/lib/email/styles.rb b/lib/email/styles.rb index 562b4982a74..9e014eef888 100644 --- a/lib/email/styles.rb +++ b/lib/email/styles.rb @@ -9,6 +9,7 @@ module Email MAX_IMAGE_DIMENSION = 400 ONEBOX_IMAGE_BASE_STYLE = "max-height: 80%; max-width: 20%; height: auto; float: left; margin-right: 10px;" ONEBOX_IMAGE_THUMBNAIL_STYLE = "width: 60px;" + ONEBOX_INLINE_AVATAR_STYLE = "width: 20px; height: 20px; float: none; vertical-align: middle;" @@plugin_callbacks = [] @@ -133,7 +134,7 @@ module Email style('.onebox-metadata', "color: #919191") style('.github-info', "margin-top: 10px;") style('.github-info div', "display: inline; margin-right: 10px;") - style('.onebox-avatar-inline', "width: 20px; height: 20px; float: none; vertical-align: middle;") + style('.onebox-avatar-inline', ONEBOX_INLINE_AVATAR_STYLE) @fragment.css('aside.quote blockquote > p').each do |p| p['style'] = 'padding: 0;' @@ -261,8 +262,10 @@ module Email if attachments[original_filename] url = attachments[original_filename].url - style = if div['data-oneboxed'] - "#{ONEBOX_IMAGE_THUMBNAIL_STYLE} #{ONEBOX_IMAGE_BASE_STYLE}" + onebox_type = div['data-onebox-type'] + style = if onebox_type + onebox_style = onebox_type == "avatar-inline" ? ONEBOX_INLINE_AVATAR_STYLE : ONEBOX_IMAGE_THUMBNAIL_STYLE + "#{onebox_style} #{ONEBOX_IMAGE_BASE_STYLE}" else calculate_width_and_height_style(div) end diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index 8dea3107554..d9bc62ad192 100644 --- a/lib/pretty_text.rb +++ b/lib/pretty_text.rb @@ -451,28 +451,36 @@ module PrettyText width = img['width'] height = img['height'] - oneboxed = img.ancestors.css(".onebox-body").any? || img.classes.include?("onebox-avatar") + onebox_type = nil + + if img.ancestors.css(".onebox-body").any? + if img.classes.include?("onebox-avatar-inline") + onebox_type = "avatar-inline" + else + onebox_type = "thumbnail" + end + end # we always want this to be tiny and without any special styles if img.classes.include?('site-icon') - oneboxed = false + onebox_type = nil width = 16 height = 16 end if Upload.secure_media_url?(url) - img.add_next_sibling secure_media_placeholder(doc, url, oneboxed: oneboxed, width: width, height: height) + img.add_next_sibling secure_media_placeholder(doc, url, onebox_type: onebox_type, width: width, height: height) img.remove end end end - def self.secure_media_placeholder(doc, url, oneboxed: false, width: nil, height: nil) + def self.secure_media_placeholder(doc, url, onebox_type: false, width: nil, height: nil) data_width = width ? "data-width=#{width}" : '' data_height = height ? "data-height=#{height}" : '' - data_oneboxed = oneboxed ? "data-oneboxed=true" : '' + data_onebox_type = onebox_type ? "data-onebox-type='#{onebox_type}'" : '' <<~HTML - <div class="secure-media-notice" data-stripped-secure-media="#{url}" #{data_oneboxed} #{data_width} #{data_height}> + <div class="secure-media-notice" data-stripped-secure-media="#{url}" #{data_onebox_type} #{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 diff --git a/spec/components/email/styles_spec.rb b/spec/components/email/styles_spec.rb index 382f1a11d64..4c1db489047 100644 --- a/spec/components/email/styles_spec.rb +++ b/spec/components/email/styles_spec.rb @@ -362,6 +362,58 @@ describe Email::Styles do expect(@frag.css('[data-embedded-secure-image]')[1].attr('style')).to eq('width: 60px; max-height: 80%; max-width: 20%; height: auto; float: left; margin-right: 10px;') end end + + context "when there is an inline-avatar in the onebox" do + let(:html) do + <<~HTML +<p><a class="mention" href="/u/martin">@martin</a> check this out:</p> +<aside class="onebox githubpullrequest"> + <header class="source"> + <a href="https://github.com/discourse/discourse/pull/11140" target="_blank" rel="noopener">github.com/discourse/discourse</a> + </header> + <article class="onebox-body"> + <div class="github-row"> + <div class="github-info-container"> + <h4> + <a href="https://github.com/discourse/discourse/pull/11140" target="_blank" rel="noopener">FEATURE: Implement edit functionality for post notices</a> + </h4> + <div class="branches"> + <code>discourse:master</code> ← <code>discourse:feature/post_notices_edit</code> + </div> + + <div class="github-info"> + <div class="date"> + opened <span class="discourse-local-date" data-format="ll" data-date="2020-11-05" data-time="20:33:53" data-timezone="UTC">08:33PM - 05 Nov 20 UTC</span> + </div> + <div class="user"> + <a href="https://github.com/udan11" target="_blank" rel="noopener"> + <img alt="udan11" src="#{Discourse.base_url}/secure-media-uploads/original/1X/123456.png" class="onebox-avatar-inline" width="20" height="20"> + udan11 + </a> + </div> + <div class="lines" title="2 commits changed 27 files with 250 additions and 224 deletions"> + <a href="https://github.com/discourse/discourse/pull/11140/files" target="_blank" rel="noopener"> + <span class="added">+250</span> + <span class="removed">-224</span> + </a> + </div> + </div> + </div> +</div> + </article> + <div class="onebox-metadata"> + </div> + <div style="clear: both"></div> +</aside> + HTML + end + it "keeps the special onebox styles" do + strip_and_inline + expect(@frag.to_s).to include("cid:email/test.png") + expect(@frag.css('[data-sripped-secure-media]')).not_to be_present + expect(@frag.css('[data-embedded-secure-image]')[0].attr('style')).to eq('width: 20px; height: 20px; float: none; vertical-align: middle; max-height: 80%; max-width: 20%; height: auto; float: left; margin-right: 10px;') + end + end end end end