From 87682f7539c63523e5a23e56b97612d1ba018203 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Thu, 17 Apr 2014 12:32:51 -0400
Subject: [PATCH] FIX: Don't include image meta data when embedded in an email

---
 app/helpers/user_notifications_helper.rb |  4 +++
 app/views/email/_post.html.erb           |  2 +-
 lib/pretty_text.rb                       | 14 ++++++++--
 spec/components/pretty_text_spec.rb      | 33 +++++++++++++++++++++---
 4 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/app/helpers/user_notifications_helper.rb b/app/helpers/user_notifications_helper.rb
index 1a5ef192364..a08366feb59 100644
--- a/app/helpers/user_notifications_helper.rb
+++ b/app/helpers/user_notifications_helper.rb
@@ -64,4 +64,8 @@ module UserNotificationsHelper
       raw Sanitize.clean(para.to_s, UserNotificationsHelper.sanitize_options)
     end
   end
+
+  def cooked_post_for_email(post)
+    PrettyText.format_for_email(post.cooked).html_safe
+  end
 end
diff --git a/app/views/email/_post.html.erb b/app/views/email/_post.html.erb
index dfa2939897f..f9fa9787df6 100644
--- a/app/views/email/_post.html.erb
+++ b/app/views/email/_post.html.erb
@@ -10,7 +10,7 @@
       </td>
     </tr>
     <tr>
-      <td class='body'><%= PrettyText.make_all_links_absolute(post.cooked).html_safe %></td>
+      <td class='body'><%= cooked_post_for_email(post) %></td>
     </tr>
   </tbody>
 </table>
diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb
index 75686b9f196..c24a3abc411 100644
--- a/lib/pretty_text.rb
+++ b/lib/pretty_text.rb
@@ -237,9 +237,9 @@ module PrettyText
     fragment.to_html
   end
 
-  def self.make_all_links_absolute(html)
+  # Given a Nokogiri doc, convert all links to absolute
+  def self.make_all_links_absolute(doc)
     site_uri = nil
-    doc = Nokogiri::HTML.fragment(html)
     doc.css("a").each do |link|
       href = link["href"].to_s
       begin
@@ -250,6 +250,16 @@ module PrettyText
         # leave it
       end
     end
+  end
+
+  def self.strip_image_wrapping(doc)
+    doc.css(".lightbox-wrapper .meta").remove
+  end
+
+  def self.format_for_email(html)
+    doc = Nokogiri::HTML.fragment(html)
+    make_all_links_absolute(doc)
+    strip_image_wrapping(doc)
     doc.to_html
   end
 
diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb
index 1e14ca93b09..03770134060 100644
--- a/spec/components/pretty_text_spec.rb
+++ b/spec/components/pretty_text_spec.rb
@@ -185,29 +185,54 @@ describe PrettyText do
   describe "make_all_links_absolute" do
     let(:base_url) { "http://baseurl.net" }
 
+    def make_abs_string(html)
+      doc = Nokogiri::HTML.fragment(html)
+      described_class.make_all_links_absolute(doc)
+      doc.to_html
+    end
+
     before do
       Discourse.stubs(:base_url).returns(base_url)
     end
 
     it "adds base url to relative links" do
       html = "<p><a class=\"mention\" href=\"/users/wiseguy\">@wiseguy</a>, <a class=\"mention\" href=\"/users/trollol\">@trollol</a> what do you guys think? </p>"
-      output = described_class.make_all_links_absolute(html)
+      output = make_abs_string(html)
       output.should == "<p><a class=\"mention\" href=\"#{base_url}/users/wiseguy\">@wiseguy</a>, <a class=\"mention\" href=\"#{base_url}/users/trollol\">@trollol</a> what do you guys think? </p>"
     end
 
     it "doesn't change external absolute links" do
       html = "<p>Check out <a href=\"http://mywebsite.com/users/boss\">this guy</a>.</p>"
-      described_class.make_all_links_absolute(html).should == html
+      make_abs_string(html).should == html
     end
 
     it "doesn't change internal absolute links" do
       html = "<p>Check out <a href=\"#{base_url}/users/boss\">this guy</a>.</p>"
-      described_class.make_all_links_absolute(html).should == html
+      make_abs_string(html).should == html
     end
 
     it "can tolerate invalid URLs" do
       html = "<p>Check out <a href=\"not a real url\">this guy</a>.</p>"
-      expect { described_class.make_all_links_absolute(html) }.to_not raise_error
+      expect { make_abs_string(html) }.to_not raise_error
+    end
+  end
+
+  describe "strip_image_wrapping" do
+    def strip_image_wrapping(html)
+      doc = Nokogiri::HTML.fragment(html)
+      described_class.strip_image_wrapping(doc)
+      doc.to_html
+    end
+
+    it "doesn't change HTML when there's no wrapped image" do
+      html = "<img src=\"wat.png\">"
+      strip_image_wrapping(html).should == html
+    end
+
+    let(:wrapped_image) { "<div class=\"lightbox-wrapper\"><a href=\"//localhost:3000/uploads/default/4399/33691397e78b4d75.png\" class=\"lightbox\" title=\"Screen Shot 2014-04-14 at 9.47.10 PM.png\"><img src=\"//localhost:3000/uploads/default/_optimized/bd9/b20/bbbcd6a0c0_655x500.png\" width=\"655\" height=\"500\"><div class=\"meta\">\n<span class=\"filename\">Screen Shot 2014-04-14 at 9.47.10 PM.png</span><span class=\"informations\">966x737 1.47 MB</span><span class=\"expand\"></span>\n</div></a></div>" }
+
+    it "strips the metadata" do
+      strip_image_wrapping(wrapped_image).should == "<div class=\"lightbox-wrapper\"><a href=\"//localhost:3000/uploads/default/4399/33691397e78b4d75.png\" class=\"lightbox\" title=\"Screen Shot 2014-04-14 at 9.47.10 PM.png\"><img src=\"//localhost:3000/uploads/default/_optimized/bd9/b20/bbbcd6a0c0_655x500.png\" width=\"655\" height=\"500\"></a></div>"
     end
   end