NEW: large image placeholder added in cooked html (#5291)

app/assets/stylesheets/common/base/topic-post.scss

@@ -414,6 +414,40 @@ a.mention, a.mention-group {
   }
 }
 
+.large-image-placeholder {
+  > a {
+
+    &.link {
+      margin-right: 10px;
+    }
+
+    > * { overflow: hidden; }
+
+    > i.fa {
+      color: dark-light-choose($primary-medium, $secondary-medium);
+      margin-right: 6px;
+      font-size: $base-font-size;
+      line-height: $base-line-height;
+    }
+
+    > span.url {
+      display: inline-block;
+      max-width: 300px;
+      margin-right: 6px;
+      text-overflow: ellipsis;
+      white-space: nowrap;
+    }
+
+    > span.help {
+      display: inline-block;
+      color: dark-light-choose($primary-medium, $secondary-medium);
+      font-size: 0.929em;
+      font-style: italic;
+      line-height: $base-line-height;
+    }
+  }
+}
+
 .broken-image, .large-image {
   color: dark-light-choose($primary-low-mid, $secondary-high);
   border: 1px solid $primary-low;

app/jobs/regular/pull_hotlinked_images.rb

@@ -8,6 +8,8 @@ module Jobs
 
     sidekiq_options queue: 'low'
 
+    LARGE_IMAGES = "large_images".freeze
+
     def initialize
       @max_size = SiteSetting.max_image_size_kb.kilobytes
     end
@@ -46,7 +48,8 @@ module Jobs
       raw = post.raw.dup
       start_raw = raw.dup
       downloaded_urls = {}
-      broken_images, large_images = [], []
+      large_images = post.custom_fields[LARGE_IMAGES].presence || []
+      broken_images, new_large_images = [], []
 
       extract_images_from(post.cooked).each do |image|
         src = original_src = image['src']
@@ -57,7 +60,7 @@ module Jobs
         if is_valid_image_url(src)
           begin
             # have we already downloaded that file?
-            unless downloaded_urls.include?(src)
+            unless downloaded_urls.include?(src) || large_images.include?(src) || broken_images.include?(src)
               if hotlinked = download(src)
                 if File.size(hotlinked.path) <= @max_size
                   filename = File.basename(URI.parse(src).path)
@@ -70,6 +73,7 @@ module Jobs
                   end
                 else
                   large_images << original_src
+                  new_large_images << original_src
                 end
               else
                 broken_images << original_src
@@ -104,15 +108,18 @@ module Jobs
 
       end
 
+      post.custom_fields[LARGE_IMAGES] = large_images
+      post.save!
       post.reload
+
       if start_raw == post.raw && raw != post.raw
         changes = { raw: raw, edit_reason: I18n.t("upload.edit_reason") }
         # we never want that job to bump the topic
         options = { bypass_bump: true }
         post.revise(Discourse.system_user, changes, options)
-      elsif downloaded_urls.present?
+      elsif downloaded_urls.present? || new_large_images.present?
         post.trigger_post_process(true)
-      elsif broken_images.present? || large_images.present?
+      elsif broken_images.present?
         start_html = post.cooked
         doc = Nokogiri::HTML::fragment(start_html)
         images = doc.css("img[src]") - doc.css("img.avatar")
@@ -125,21 +132,6 @@ module Jobs
             tag.remove_attribute('src')
             tag.remove_attribute('width')
             tag.remove_attribute('height')
-          elsif large_images.include?(src)
-            tag.name = 'a'
-            tag.set_attribute('href', src)
-            tag.set_attribute('target', '_blank')
-            tag.set_attribute('title', I18n.t('post.image_placeholder.large'))
-            tag.remove_attribute('src')
-            tag.remove_attribute('width')
-            tag.remove_attribute('height')
-            tag.inner_html = '<span class="large-image fa fa-picture-o"></span>'
-            parent = tag.parent
-            if parent.name == 'a'
-              parent.add_next_sibling(tag)
-              parent.add_next_sibling('<br>')
-              parent.content = parent["href"]
-            end
           end
         end
         if start_html == post.cooked && doc.to_html != post.cooked

config/locales/server.en.yml

@@ -524,7 +524,6 @@ en:
   post:
     image_placeholder:
       broken: "This image is broken"
-      large: "This image is too large to display here; click to view"
 
   rate_limiter:
     slow_down: "You have performed this action too many times, try again later."
@@ -2870,6 +2869,8 @@ en:
       too_large: "Sorry, the image you are trying to upload is too big (maximum size is %{max_size_kb}KB), please resize it and try again."
       larger_than_x_megapixels: "Sorry, the image you are trying to upload is too large (maximum dimension is %{max_image_megapixels}-megapixels), please resize it and try again."
       size_not_found: "Sorry, but we couldn't determine the size of the image. Maybe your image is corrupted?"
+    placeholders:
+      too_large: "(image larger than %{max_size_kb}KB)"
 
   avatar:
     missing: "Sorry, we can't find any avatar associated with that email address. Can you try uploading it again?"

lib/cooked_post_processor.rb

@@ -81,11 +81,53 @@ class CookedPostProcessor
     return if images.blank?
 
     images.each do |img|
+      next if large_images.include?(img["src"]) && add_large_image_placeholder!(img)
+
       limit_size!(img)
       convert_to_link!(img)
     end
   end
 
+  def add_large_image_placeholder!(img)
+    url = img["src"]
+
+    is_hyperlinked = is_a_hyperlink?(img)
+
+    placeholder = create_node("div", "large-image-placeholder")
+    img.add_next_sibling(placeholder)
+    placeholder.add_child(img)
+
+    a = create_link_node(nil, url, true)
+    img.add_next_sibling(a)
+
+    span = create_span_node("url", url)
+    a.add_child(span)
+    span.add_previous_sibling(create_icon_node("image"))
+    span.add_next_sibling(create_span_node("help", I18n.t("upload.placeholders.too_large", max_size_kb: SiteSetting.max_image_size_kb)))
+
+    # Only if the image is already linked
+    if is_hyperlinked
+      parent = placeholder.parent
+      parent.add_next_sibling(placeholder)
+
+      if parent.name == 'a' && parent["href"].present? && url != parent["href"]
+        parent["class"] = "link"
+        a.add_previous_sibling(parent)
+
+        lspan = create_span_node("url", parent["href"])
+        parent.add_child(lspan)
+        lspan.add_previous_sibling(create_icon_node("link"))
+      end
+    end
+
+    img.remove
+    true
+  end
+
+  def large_images
+    @large_images ||= @post.custom_fields[Jobs::PullHotlinkedImages::LARGE_IMAGES].presence || []
+  end
+
   def extract_images
     # all image with a src attribute
     @doc.css("img[src]") -
@@ -244,21 +286,18 @@ class CookedPostProcessor
 
   def add_lightbox!(img, original_width, original_height, upload = nil)
     # first, create a div to hold our lightbox
-    lightbox = Nokogiri::XML::Node.new("div", @doc)
+    lightbox = create_node("div", "lightbox-wrapper")
-    lightbox["class"] = "lightbox-wrapper"
     img.add_next_sibling(lightbox)
     lightbox.add_child(img)
 
     # then, the link to our larger image
-    a = Nokogiri::XML::Node.new("a", @doc)
+    a = create_link_node("lightbox", img["src"])
     img.add_next_sibling(a)
 
     if upload && Discourse.store.internal?
       a["data-download-href"] = Discourse.store.download_url(upload)
     end
 
-    a["href"] = img["src"]
-    a["class"] = "lightbox"
     a.add_child(img)
 
     # replace the image by its thumbnail
@@ -266,8 +305,7 @@ class CookedPostProcessor
     img["src"] = upload.thumbnail(w, h).url if upload && upload.has_thumbnail?(w, h)
 
     # then, some overlay informations
-    meta = Nokogiri::XML::Node.new("div", @doc)
+    meta = create_node("div", "meta")
-    meta["class"] = "meta"
     img.add_next_sibling(meta)
 
     filename = get_filename(upload, img["src"])
@@ -287,13 +325,32 @@ class CookedPostProcessor
     return I18n.t("upload.pasted_image_filename")
   end
 
+  def create_node(tag_name, klass)
+    node = Nokogiri::XML::Node.new(tag_name, @doc)
+    node["class"] = klass if klass.present?
+    node
+  end
+
   def create_span_node(klass, content = nil)
-    span = Nokogiri::XML::Node.new("span", @doc)
+    span = create_node("span", klass)
     span.content = content if content
-    span["class"] = klass
     span
   end
 
+  def create_icon_node(klass)
+    create_node("i", "fa fa-fw fa-#{klass}")
+  end
+
+  def create_link_node(klass, url, external = false)
+    a = create_node("a", klass)
+    a["href"] = url
+    if external
+      a["target"] = "_blank"
+      a["rel"] = "nofollow noopener"
+    end
+    a
+  end
+
   def update_post_image
     img = extract_images_for_post.first
     return if img.blank?
@@ -318,14 +375,17 @@ class CookedPostProcessor
 
     uploads = oneboxed_image_uploads.select(:url, :origin)
     oneboxed_images.each do |img|
+      if large_images.include?(img["src"])
+        img.remove
+        next
+      end
+
       url = img["src"].sub(/^https?:/i, "")
       upload = uploads.find { |u| u.origin.sub(/^https?:/i, "") == url }
       img["src"] = upload.url if upload.present?
-    end
 
       # make sure we grab dimensions for oneboxed images
       # and wrap in a div
-    oneboxed_images.each do |img|
       limit_size!(img)
 
       next if img["class"]&.include?('onebox-avatar')

spec/components/cooked_post_processor_spec.rb

@@ -162,7 +162,7 @@ describe CookedPostProcessor do
 
       it "generates overlay information" do
         cpp.post_process_images
-        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a data-download-href=\"/uploads/default/#{upload.sha1}\" href=\"/uploads/default/1/1234567890123456.jpg\" class=\"lightbox\" title=\"logo.png\"><img src=\"/uploads/default/optimized/1X/#{upload.sha1}_1_690x788.png\" width=\"690\" height=\"788\"><div class=\"meta\">
+        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a class=\"lightbox\" href=\"/uploads/default/1/1234567890123456.jpg\" data-download-href=\"/uploads/default/#{upload.sha1}\" title=\"logo.png\"><img src=\"/uploads/default/optimized/1X/#{upload.sha1}_1_690x788.png\" width=\"690\" height=\"788\"><div class=\"meta\">
 <span class=\"filename\">logo.png</span><span class=\"informations\">1750x2000 1.21 KB</span><span class=\"expand\"></span>
 </div></a></div></p>"
         expect(cpp).to be_dirty
@@ -245,7 +245,7 @@ describe CookedPostProcessor do
 
       it "generates overlay information" do
         cpp.post_process_images
-        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a data-download-href=\"/subfolder/uploads/default/#{upload.sha1}\" href=\"/subfolder/uploads/default/1/1234567890123456.jpg\" class=\"lightbox\" title=\"logo.png\"><img src=\"/subfolder/uploads/default/optimized/1X/#{upload.sha1}_1_690x788.png\" width=\"690\" height=\"788\"><div class=\"meta\">
+        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a class=\"lightbox\" href=\"/subfolder/uploads/default/1/1234567890123456.jpg\" data-download-href=\"/subfolder/uploads/default/#{upload.sha1}\" title=\"logo.png\"><img src=\"/subfolder/uploads/default/optimized/1X/#{upload.sha1}_1_690x788.png\" width=\"690\" height=\"788\"><div class=\"meta\">
 <span class=\"filename\">logo.png</span><span class=\"informations\">1750x2000 1.21 KB</span><span class=\"expand\"></span>
 </div></a></div></p>"
         expect(cpp).to be_dirty
@@ -254,7 +254,7 @@ describe CookedPostProcessor do
       it "should escape the filename" do
         upload.update_attributes!(original_filename: "><img src=x onerror=alert('haha')>.png")
         cpp.post_process_images
-        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a data-download-href=\"/subfolder/uploads/default/#{upload.sha1}\" href=\"/subfolder/uploads/default/1/1234567890123456.jpg\" class=\"lightbox\" title=\"&amp;gt;&amp;lt;img src=x onerror=alert(&amp;#39;haha&amp;#39;)&amp;gt;.png\"><img src=\"/subfolder/uploads/default/optimized/1X/#{upload.sha1}_1_690x788.png\" width=\"690\" height=\"788\"><div class=\"meta\">
+        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a class=\"lightbox\" href=\"/subfolder/uploads/default/1/1234567890123456.jpg\" data-download-href=\"/subfolder/uploads/default/#{upload.sha1}\" title=\"&amp;gt;&amp;lt;img src=x onerror=alert(&amp;#39;haha&amp;#39;)&amp;gt;.png\"><img src=\"/subfolder/uploads/default/optimized/1X/#{upload.sha1}_1_690x788.png\" width=\"690\" height=\"788\"><div class=\"meta\">
 <span class=\"filename\">&amp;gt;&amp;lt;img src=x onerror=alert(&amp;#39;haha&amp;#39;)&amp;gt;.png</span><span class=\"informations\">1750x2000 1.21 KB</span><span class=\"expand\"></span>
 </div></a></div></p>"
       end
@@ -280,7 +280,7 @@ describe CookedPostProcessor do
 
       it "generates overlay information" do
         cpp.post_process_images
-        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a data-download-href=\"/uploads/default/#{upload.sha1}\" href=\"/uploads/default/1/1234567890123456.jpg\" class=\"lightbox\" title=\"WAT\"><img src=\"/uploads/default/optimized/1X/#{upload.sha1}_1_690x788.png\" title=\"WAT\" width=\"690\" height=\"788\"><div class=\"meta\">
+        expect(cpp.html).to match_html "<p><div class=\"lightbox-wrapper\"><a class=\"lightbox\" href=\"/uploads/default/1/1234567890123456.jpg\" data-download-href=\"/uploads/default/#{upload.sha1}\" title=\"WAT\"><img src=\"/uploads/default/optimized/1X/#{upload.sha1}_1_690x788.png\" title=\"WAT\" width=\"690\" height=\"788\"><div class=\"meta\">
        <span class=\"filename\">WAT</span><span class=\"informations\">1750x2000 1.21 KB</span><span class=\"expand\"></span>
        </div></a></div></p>"
         expect(cpp).to be_dirty

spec/jobs/pull_hotlinked_images_spec.rb

@@ -111,7 +111,7 @@ describe Jobs::PullHotlinkedImages do
         expect(post.cooked).to match(/<p><img src=.*\/uploads/)
         expect(post.cooked).to match(/<img src=.*\/uploads.*\ class="thumbnail"/)
         expect(post.cooked).to match(/<span class="broken-image fa fa-chain-broken/)
-        expect(post.cooked).to match(/<\/a><br><a href=.*\ target="_blank" .*\><span class="large-image fa fa-picture-o"><\/span><\/a>/)
+        expect(post.cooked).to match(/<div class="large-image-placeholder">/)
       end
     end
   end
@@ -132,9 +132,10 @@ describe Jobs::PullHotlinkedImages do
 
       Jobs::ProcessPost.new.execute(post_id: post.id)
       Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
+      Jobs::ProcessPost.new.execute(post_id: post.id)
       post.reload
 
-      expect(post.cooked).to match(/<a href=.*\ target="_blank" .*\><span class="large-image fa fa-picture-o"><\/span><\/a>/)
+      expect(post.cooked).to match(/<div class="large-image-placeholder"><a href=.*\ target="_blank" .*\>/)
     end
   end