FIX: Emoji sizes in emails should be done by the EmailStyler

config/locales/server.en.yml

@@ -725,6 +725,8 @@ en:
 
       Your friends at [Discourse](http://www.discourse.org)
 
+      :smile:
+
       [0]: %{base_url}
       [1]: http://www.kitterman.com/spf/validate.html
       [2]: http://mxtoolbox.com/SuperTool.aspx

lib/email/renderer.rb

@@ -21,16 +21,19 @@ module Email
     end
 
     def html
-      cooked = PrettyText.cook(text)
+      style = Email::Styles.new(PrettyText.cook(text))
+      style.format_basic
 
       if @opts[:html_template]
+        style.format_html
+
         ActionView::Base.new(Rails.configuration.paths["app/views"]).render(
           template: 'email/template',
           format: :html,
-          locals: { html_body: Email::Styles.new(cooked).format, logo_url: logo_url }
+          locals: { html_body: style.to_html, logo_url: logo_url }
         )
       else
-        cooked
+        style.to_html
       end
     end
 

lib/email/styles.rb

@@ -7,44 +7,58 @@ module Email
 
     def initialize(html)
       @html = html
+      @fragment = Nokogiri::HTML.fragment(@html)
     end
 
-    def format
+    def format_basic
-      fragment = Nokogiri::HTML.fragment(@html)
+      @fragment.css('img').each do |img|
 
-      fragment.css('h3').each do |h3|
+        if img['src'] =~ /\/assets\/emoji\//
+          img['style'] = "width: 20px; height: 20px;"
+        else
+          img['style'] = "max-width: 694px;"
+        end
+
+        if img['src'][0] == "/"
+          img['src'] = "#{Discourse.base_url}#{img['src']}"
+        end
+
+      end
+    end
+
+    def format_html
+      @fragment.css('h3').each do |h3|
         h3['style'] = 'margin: 15px 0 20px 0; border-bottom: 1px solid #ddd;'
       end
 
-      fragment.css('hr').each do |hr|
+      @fragment.css('hr').each do |hr|
         hr['style'] = 'background-color: #ddd; height: 1px; border: 1px;'
       end
 
-      fragment.css('img').each do |img|
-        img['style'] = "max-width: 694px;"
-      end
 
-      fragment.css('a').each do |a|
+      @fragment.css('a').each do |a|
         a['style'] = 'text-decoration: none; font-weight: bold; font-size: 15px; color: #006699;'
       end
 
-      fragment.css('ul').each do |ul|
+      @fragment.css('ul').each do |ul|
         ul['style'] = 'margin: 0 0 0 10px; padding: 0 0 0 20px;'
       end
 
-      fragment.css('li').each do |li|
+      @fragment.css('li').each do |li|
         li['style'] = 'padding-bottom: 10px'
       end
 
-      fragment.css('pre').each do |pre|
+      @fragment.css('pre').each do |pre|
         pre.replace(pre.text)
       end
 
-      fragment.css('div.digest-post').each do |div|
+      @fragment.css('div.digest-post').each do |div|
         div['style'] = 'margin-left: 15px; margin-top: 20px; max-width: 694px;'
       end
+    end
 
-      fragment.to_html
+    def to_html
+      @fragment.to_html
     end
 
 

spec/components/email/email_styles_spec.rb

@@ -3,38 +3,85 @@ require 'email'
 
 describe Email::Styles do
 
-  def style_exists(html, css_rule)
+  def basic_fragment(html)
-    fragment = Nokogiri::HTML.fragment(Email::Styles.new(html).format)
+    styler = Email::Styles.new(html)
-    element = fragment.at(css_rule)
+    styler.format_basic
-    expect(element["style"]).not_to be_blank
+    Nokogiri::HTML.fragment(styler.to_html)
   end
 
-  it "returns blank from an empty string" do
+  def html_fragment(html)
-    Email::Styles.new("").format.should be_blank
+    styler = Email::Styles.new(html)
+    styler.format_basic
+    styler.format_html
+    Nokogiri::HTML.fragment(styler.to_html)
   end
 
-  it "attaches a style to h3 tags" do
+  context "basic formatter" do
-    style_exists("<h3>hello</h3>", "h3")
+
+    it "works with an empty string" do
+      style = Email::Styles.new("")
+      style.format_basic
+      expect(style.to_html).to be_blank
+    end
+
+    it "adds a max-width to images" do
+      frag = basic_fragment("<img src='gigantic.jpg'>")
+      expect(frag.at("img")["style"]).to match("max-width")
+    end
+
+    it "adds a width and height to images with an emoji path" do
+      frag = basic_fragment("<img src='/assets/emoji/fish.png'>")
+      expect(frag.at("img")["style"]).to match("width:")
+      expect(frag.at("img")["style"]).to match("height:")
+    end
+
+    it "converts relative paths to absolute paths" do
+      frag = basic_fragment("<img src='/some-image.png'>")
+      expect(frag.at("img")["src"]).to eq("#{Discourse.base_url}/some-image.png")
+    end
+
   end
 
-  it "attaches a style to hr tags" do
+  context "html template formatter" do
-    style_exists("hello<hr>", "hr")
+    it "works with an empty string" do
+      style = Email::Styles.new("")
+      style.format_html
+      expect(style.to_html).to be_blank
+    end
+
+    it "attaches a style to h3 tags" do
+      frag = html_fragment("<h3>hello</h3>")
+      expect(frag.at('h3')['style']).to be_present
+    end
+
+    it "attaches a style to hr tags" do
+      frag = html_fragment("hello<hr>")
+      expect(frag.at('hr')['style']).to be_present
+    end
+
+    it "attaches a style to a tags" do
+      frag = html_fragment("<a href='#'>wat</a>")
+      expect(frag.at('a')['style']).to be_present
+    end
+
+    it "attaches a style to a tags" do
+      frag = html_fragment("<a href='#'>wat</a>")
+      expect(frag.at('a')['style']).to be_present
+    end
+
+    it "attaches a style to ul and li tags" do
+      frag = html_fragment("<ul><li>hello</li></ul>")
+      expect(frag.at('ul')['style']).to be_present
+      expect(frag.at('li')['style']).to be_present
+    end
+
+    it "removes pre tags but keeps their contents" do
+      style = Email::Styles.new("<pre>hello</pre>")
+      style.format_basic
+      style.format_html
+      expect(style.to_html).to eq("hello")
+    end
   end
 
-  it "attaches a style to a tags" do
-    style_exists("<a href='#'>wat</a>", "a")
-  end
-
-  it "attaches a style to ul tags" do
-    style_exists("<ul><li>hello</li></ul>", "ul")
-  end
-
-  it "attaches a style to li tags" do
-    style_exists("<ul><li>hello</li></ul>", "li")
-  end
-
-  it "removes pre tags but keeps their contents" do
-    expect(Email::Styles.new("<pre>hello</pre>").format).to eq("hello")
-  end
 
 end

vendor/gems/discourse_emoji/vendor/assets/javascripts/discourse_emoji.js

@@ -7,16 +7,10 @@
     var text = event.detail;
     var opts = event.opts;
 
-    style = ""
-    if (opts && opts.environment === "email") {
-      // Hard code sizes for email view
-      style = 'width="20" height="20"';
-    }
-
     this.textResult = text.replace(/\:([a-z\_\+\-0-9]+)\:/g, function (m1, m2) {
       var url = Discourse.getURL('/assets/emoji/' + m2 + '.png');
       return (emoji.indexOf(m2) !== -1) ?
-             '<img alt="' + m2 + '" title=":' + m2 + ':" src="' + url + '" ' + style + ' class="emoji"/>' :
+             '<img alt="' + m2 + '" title=":' + m2 + ':" src="' + url + '" class="emoji"/>' :
              m1;
     });
   });