FEATURE: Add placeholders to broken and large image files (#5113)

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

@@ -420,3 +420,9 @@ a.mention, a.mention-group {
     padding-bottom: 15px;
   }
 }
+
+.broken-image, .large-image {
+  border: 1px solid $primary-low;
+  font-size: 32px;
+  padding: 16px;
+}

app/jobs/regular/pull_hotlinked_images.rb

@@ -25,6 +25,7 @@ module Jobs
       raw = post.raw.dup
       start_raw = raw.dup
       downloaded_urls = {}
+      broken_images, large_images = [], []
 
       extract_images_from(post.cooked).each do |image|
         src = original_src = image['src']
@@ -56,9 +57,11 @@ module Jobs
                   end
                 else
                   log(:info, "Failed to pull hotlinked image for post: #{post_id}: #{src} - Image is bigger than #{@max_size}")
+                  large_images << original_src
                 end
               else
                 log(:info, "There was an error while downloading '#{src}' locally for post: #{post_id}")
+                broken_images << original_src
               end
             end
             # have we successfully downloaded that file?
@@ -98,6 +101,28 @@ module Jobs
         post.revise(Discourse.system_user, changes, options)
       elsif downloaded_urls.present?
         post.trigger_post_process(true)
+      elsif broken_images.present? || large_images.present?
+        start_html = post.cooked
+        doc = Nokogiri::HTML::fragment(start_html)
+        images = doc.css("img[src]") - doc.css("img.avatar")
+        images.each do |tag|
+          src = tag['src']
+          if broken_images.include?(src)
+            tag.name = 'span'
+            tag.set_attribute('class', 'broken-image fa fa-chain-broken')
+            tag.remove_attribute('src')
+          elsif large_images.include?(src)
+            tag.name = 'a'
+            tag.set_attribute('href', src)
+            tag.set_attribute('target', '_blank')
+            tag.remove_attribute('src')
+            tag.inner_html = '<span class="large-image fa fa-picture-o"></span>'
+          end
+        end
+        if start_html == post.cooked && doc.to_html != post.cooked
+          post.update_column(:cooked, doc.to_html)
+          post.publish_change_to_clients! :revised
+        end
       end
     end
 

spec/jobs/pull_hotlinked_images_spec.rb

@@ -3,19 +3,29 @@ require 'jobs/regular/pull_hotlinked_images'
 
 describe Jobs::PullHotlinkedImages do
 
-  describe '#execute' do
+  let(:image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat1.png" }
-    let(:image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat1.png" }
+  let(:broken_image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat2.png" }
-    let(:png) { Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") }
+  let(:large_image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat3.png" }
+  let(:png) { Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") }
+  let(:large_png) { Base64.decode64("iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAIAAAAlC+aJAAAK10lEQVR42r3aeVRTVx4H8Oc2atWO7Sw9OnM6HWvrOON0aFlcAZ3RopZWOyqgoCACKqPWBUVQi4gIqAVllciiKPu+JOyGnQQSNgkIIQgoKljAYVARCZnf4yXhkeXlJmDP+f4hOUF+n3fvffe++y5W0i4qJqWoDU8hKQUPxWFKcq9VnHxJ8gTi5EqS0yJOtiRZfHEyJWE0i0MnJaMJTzopaQ/wpJKS0ogneTQYABANTDlDvpxBCsiu72eUP0zPq8Fzr45e8TircRDFQAAy5ABpcgDCgJV2iCbRQM+rinU/E26ie9NgfrDO1GBtTBy96SH/WhBhaxwfGEjndmfKGeiaGsYAJXIANQyCkfR05u3dhuOKVhLamnmRzocyKp9mNo9QG9IRDDiAiMaG3Nqfo45aoJROzk3DDxNCbjGahBM0yAKoDfIDOpNZE/bNYrVKJyfylB2D91pdA3lAjwE0MDAyS+BCalw9kdu2xvT6AY0NWBkJoNaAzsrj4CN1YtUTidi/hdH4BvGmJGPAAYgGMuMery/U6ONJqZ5I1PlTjNExre7kgJU/EqEbJC0gjDpiiv9hnSkJ2z+t9dzxwNcSUudlUuuxnXP+W/bZTWWO64uO6hccWQ0pPm4IP1a6GFe5bYXvNF7f0xxg3XrzgCDYjn1m4+218/D/SndaYnSqBpMDDlDXkHYnMlh7Srj+HLanxfOsyyOVN0ScYI0zkOeVZvYZGEI2/DFDMkWgTw7jAGWUA5owMOt7QtcvDF09qybA/mGC6zA7aCLVExkq9U3895/wm9LpgyonBxmDGKDQoHBySPQ8B5e/zM2kJdalN/fqxKsn8oLhFr5mdvDyX6UVNqqcpMmDAWNJACjtUMDrDVn7m6SdS/kxPwrizg+zAycLAKm5tA0a4a7DPpSFhmIAxWAgDKm0IJrutBr/g3D5n9E9J7F6oiNFGf2WtnI2vboH3YADEA0AuG2ml2i2BC4/AAYKr00uAHL/ihk0QnxQMPqKFWM/FiEamFWPYMHD8tgF1UMmZfjKZLDIJ1z/vQibzTKrbop2wAGIhoxbt8IN5zZHnoHqO5LdJr16IkXHDG4afJDJG0B8chADUAxxTnbp1trE5Z/0ASDN09hTcJdLy+EoawQZgyyAwhCxcznr0k4C0JNz5R0BYFqM3PBhQugtxKdQrEICUGFoE4ZtWPAg4jQBeJHv/Y4AkBKHdTHuZ8lP0hSDAQdQGwhAUUNv4s6/EvcfSD/T590B2u8cj3SwltkNUGaQBSgbDAXc9pxTW4jqIf8ruAa37efJLg/DfuBd21ftYU7OA387+QXSk2gHWMmRw/M2F9D2d8WffsW8Sv5+X/mtyBN7s+V2NBQasMpOEYqhuLG3MimMqL4h/GTu4fW01b/z05qrMKEGC96W+8sA8g/qKX281JuWafX350lniG++rIpOTcknb8lQGHAAoqG+pgqqr7hqE2K4kCg0bO3CJDMthvVKInTrlUmm/4j+9vO7mxYNlfrJAJiHVsYaL0g1XZy194scmy+JMCyXxWz+CAD4anTFjLrLpiMVQW+4t1G2lQiDGIBiuF/NLbmwM1B3PpQe892SFtqh4fIAhZ14mBUo34WE7ECFC29hRdDz5LO5dtrwdAGM0pP/HKoMzWsZRtwakwVQGPJjo/2/ej9Q74N8xy19o+tQYcWNzjT3mJNmR/W/uPi9fobr3ifpl6hXeG9Zge1JF5LPWvz4zYoTa7VSzu0mniggMEigNcBQ7GjE5A9Kt/eoOxLGkQBUGkoyGeEbPqnys2+OPlcbdir80PdOX+usmDFdG8OIwCc3bI0vm657WeSrsPouhuelbQZh/9nqY7FB+lsGc2ad27w86oTJo5SLrwu9s/dpVXuYFPEHELcocQC1QXpjhS4EpcMwiPhh2/U9XzfedYYFhe7UKdJSqkNOIt4oMy/uIwP68n6C3/WzMmIFHIUeJawMLm7ul9lmVdYOYgCKob6aK72NEo8yQ+UBtl99BkXoTMFcv1sF3UNaIpd24vCqvykDvCr2PbJ6GQFwNtKFrjhuCHFCCvmvcuW2ihUaMO4TWYCyAU0GSJcSsCblRTjDSJAZoFnuNiafLqReMrQlukKTylQvBZC3iikMOIDCQGaQAT9nq1gLqQRQBABFLa9U7tcTBjEApR3IALh1/DIAlQZZAIWBDOjO9HrXAMT3JliVBKCyHciALsYvAUAx4IAqOYDCmxKPBFD5QDNBQHHLS2XvfmQMYgCKgQx4muGhFmCw1B8dIOTQyvj9FO+vyDclrPqpLECZgVczBoAlA3URMCubLv6D9I657ZOP0lws1QJQv4OTGnAAogEdAF+A+TXHw3b0R5qoszLLyx4+gc8RAeUt/SrfIxIGMYDCoBDwONVdaQ9mB+3XWeK87kvJ1EYTDfYLn9XDgsdO+3NYKSACUN6FQsYAKg2IgIqgY6tnzmi6bP8y2X2EmGUbkkWCPJitV82cURfuqPq5nhPM4vchvpDGauQAygxkAMW+ULCdsfWSj/tCTr8IdeqPdBnK94FnFCEr8DXd68CyRXeObkfpRWx+D+JLdRxANlC0QwMaINHZfP37c4oczQkDnjDnvlCnMuc9RvPnxp/ehQKokAAoOlIeGUDdDvKAtsQLyv72mzJ/P6uN+rNnHtf5S7GjRVeQQ6nTbge9pdB/vEzWDso9aqoEUBuw2mciZY0gY0AEEBHEuZzZqAdFG743c/n0aQ7rtBruOKO/y+HwnyMebsABiIbG2jFAa7wryh4bPDaUXD+swWuoKv5TxMMNYgCFgQSoIgHOv7uNLbgLcfldiAc0xgAqDbVtLwTJXgQAeojmLzLKAzjBxyl257vqcgsfChUeDJA3YHUkgEpDQz2vJU7cCDJTEnQSWOHBDK0wMACgL0U7mLptXWO/fGmCk7myGW2gOra09Q36aSUcoIahc4Rfmi59JBi3H5j3k5fJOs8dhgoTYL0Jqi/1PfyMTrUKHOKGcwS9Kg9okA1iALqh+tGggBFIGJRtn2gWWEHwmlsRD5lIDdj9LpG8gXpyuN/yRJBwEQCwRYWytkEcuB28iuK2EXVPXOEAqaEW2dBUzZI+HE/wTT2RnjpGSZtQg1NjYoDa7dA50sKMIgywyTPB6l9VRbPaXmt28m0MQNEOCgdDbXu/IM17tCO5TaQjveWG1Qi6NT75htWTAOoaeA/4gnhXlF0Wiq7f3NSk1okrGQMO0NzQOdLMziU60usSPw2q7+SVlnWMlE3g1BjG6xZNxFDe1s2OO0Z0JHhxBuMBJlroUSgju682ldUxTH24QaVhDFAvB1Bp4HS+PRO/5ZDP7xtjnaXLJGKlBMtVeGqDuRk2If97z/tl0XVYZg+T3nF0F3tcjN1W2vFWrdNK8gYcgGiQvykFFl7a7oFBvG5o5UfvVRQrRuQu+mjgH5lRu7JjLPISLAtTrJ1pf94dj4U0+mhw4opsEAPU6kiEIZ1XYnZlFgFQKzu8MYtYzKYUs63E7Lnz0ls5iKeVFBrGAGq1A6uj1zZw0XZPzPwuZhqE7biiqm4vzNQP/7JVFmZbgdlxxnKienFBe4/G7YA1kADI7TDilmQJZVlE41cRirBlYdZMzIqB7UnGdseRkohZZmDW+ZhNmfibEHvuzAOcaWTD5XpLuBepdfKtiAxQ1xDPTdnhOdXUH7Nlj7uWKDnAme7bvPlI1a/Hfz4ljp+BfnqPPKD/DzQWIVWNoUiJAAAAAElFTkSuQmCC") }
 
+  before do
+    stub_request(:get, image_url).to_return(body: png, headers: { "Content-Type" => "image/png" })
+    stub_request(:head, image_url)
+    stub_request(:head, broken_image_url).to_return(status: 404)
+    stub_request(:get, large_image_url).to_return(body: large_png, headers: { "Content-Type" => "image/png" })
+    stub_request(:head, large_image_url)
+    SiteSetting.download_remote_images_to_local = true
+    SiteSetting.max_image_size_kb = 2
+  end
+
+  describe '#execute' do
     before do
-      stub_request(:get, image_url).to_return(body: png, headers: { "Content-Type" => "image/png" })
-      stub_request(:head, image_url)
-      SiteSetting.download_remote_images_to_local = true
       FastImage.expects(:size).returns([100, 100]).at_least_once
     end
 
     it 'replaces images' do
-      post = Fabricate(:post, raw: "<img src='http://wiki.mozilla.org/images/2/2e/Longcat1.png'>")
+      post = Fabricate(:post, raw: "<img src='#{image_url}'>")
 
       Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
       post.reload
@@ -24,7 +34,8 @@ describe Jobs::PullHotlinkedImages do
     end
 
     it 'replaces images without protocol' do
-      post = Fabricate(:post, raw: "<img src='//wiki.mozilla.org/images/2/2e/Longcat1.png'>")
+      url = image_url.sub(/^https?\:/, '')
+      post = Fabricate(:post, raw: "<img src='#{url}'>")
 
       Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
       post.reload
@@ -33,10 +44,10 @@ describe Jobs::PullHotlinkedImages do
     end
 
     it 'replaces images without extension' do
-      extensionless_url = "http://wiki.mozilla.org/images/2/2e/Longcat1"
+      url = image_url.sub(/\.[a-zA-Z0-9]+$/, '')
-      stub_request(:get, extensionless_url).to_return(body: png, headers: { "Content-Type" => "image/png" })
+      stub_request(:get, url).to_return(body: png, headers: { "Content-Type" => "image/png" })
-      stub_request(:head, extensionless_url)
+      stub_request(:head, url)
-      post = Fabricate(:post, raw: "<img src='#{extensionless_url}'>")
+      post = Fabricate(:post, raw: "<img src='#{url}'>")
 
       Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
       post.reload
@@ -80,6 +91,48 @@ describe Jobs::PullHotlinkedImages do
 
         expect(post.cooked).to match(/<img src=.*\/uploads/)
       end
+
+      it 'all combinations' do
+        post = Fabricate(:post, raw: "
+<img src='#{image_url}'>
+#{url}
+<img src='#{broken_image_url}'>
+<img src='#{large_image_url}'>
+        ")
+
+        Jobs::ProcessPost.new.execute(post_id: post.id)
+        Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
+        Jobs::ProcessPost.new.execute(post_id: post.id)
+        Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
+        post.reload
+
+        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 .*\ href=.*\ target="_blank"><span class="large-image fa fa-picture-o"><\/span><\/a>/)
+      end
+    end
+  end
+
+  describe 'replace' do
+    it 'broken image with placeholder' do
+      post = Fabricate(:post, raw: "<img src='#{broken_image_url}'>")
+
+      Jobs::ProcessPost.new.execute(post_id: post.id)
+      Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
+      post.reload
+
+      expect(post.cooked).to match(/<span .*\ class="broken-image fa fa-chain-broken/)
+    end
+
+    it 'large image with placeholder' do
+      post = Fabricate(:post, raw: "<img src='#{large_image_url}'>")
+
+      Jobs::ProcessPost.new.execute(post_id: post.id)
+      Jobs::PullHotlinkedImages.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>/)
     end
   end