From 66f5a3f6f6a63e92e115d60b04069424822e40e2 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 13 Jun 2013 12:15:05 -0400 Subject: [PATCH] FIX: Emoji sizes in emails should be done by the EmailStyler --- config/locales/server.en.yml | 2 + lib/email/renderer.rb | 9 +- lib/email/styles.rb | 40 +++++--- spec/components/email/email_styles_spec.rb | 97 ++++++++++++++----- .../assets/javascripts/discourse_emoji.js | 8 +- 5 files changed, 108 insertions(+), 48 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index eb25e477e76..b0ac0896aef 100644 --- a/config/locales/server.en.yml +++ b/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 diff --git a/lib/email/renderer.rb b/lib/email/renderer.rb index 24ac6680ec2..61894888492 100644 --- a/lib/email/renderer.rb +++ b/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 diff --git a/lib/email/styles.rb b/lib/email/styles.rb index f9f466c3ffb..9bba50bc6e6 100644 --- a/lib/email/styles.rb +++ b/lib/email/styles.rb @@ -7,44 +7,58 @@ module Email def initialize(html) @html = html + @fragment = Nokogiri::HTML.fragment(@html) end - def format - fragment = Nokogiri::HTML.fragment(@html) + def format_basic + @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 diff --git a/spec/components/email/email_styles_spec.rb b/spec/components/email/email_styles_spec.rb index 8608d07218b..b1b7d8c7503 100644 --- a/spec/components/email/email_styles_spec.rb +++ b/spec/components/email/email_styles_spec.rb @@ -3,38 +3,85 @@ require 'email' describe Email::Styles do - def style_exists(html, css_rule) - fragment = Nokogiri::HTML.fragment(Email::Styles.new(html).format) - element = fragment.at(css_rule) - expect(element["style"]).not_to be_blank + def basic_fragment(html) + styler = Email::Styles.new(html) + styler.format_basic + Nokogiri::HTML.fragment(styler.to_html) end - it "returns blank from an empty string" do - Email::Styles.new("").format.should be_blank + def html_fragment(html) + 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 - style_exists("

hello

", "h3") + context "basic formatter" do + + 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("") + 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("") + 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("") + expect(frag.at("img")["src"]).to eq("#{Discourse.base_url}/some-image.png") + end + end - it "attaches a style to hr tags" do - style_exists("hello
", "hr") + context "html template formatter" do + 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("

hello

") + expect(frag.at('h3')['style']).to be_present + end + + it "attaches a style to hr tags" do + frag = html_fragment("hello
") + expect(frag.at('hr')['style']).to be_present + end + + it "attaches a style to a tags" do + frag = html_fragment("wat") + expect(frag.at('a')['style']).to be_present + end + + it "attaches a style to a tags" do + frag = html_fragment("wat") + expect(frag.at('a')['style']).to be_present + end + + it "attaches a style to ul and li tags" do + frag = html_fragment("") + 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("
hello
") + 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("wat", "a") - end - - it "attaches a style to ul tags" do - style_exists("", "ul") - end - - it "attaches a style to li tags" do - style_exists("", "li") - end - - it "removes pre tags but keeps their contents" do - expect(Email::Styles.new("
hello
").format).to eq("hello") - end end diff --git a/vendor/gems/discourse_emoji/vendor/assets/javascripts/discourse_emoji.js b/vendor/gems/discourse_emoji/vendor/assets/javascripts/discourse_emoji.js index cfbc16d457c..98614284f7d 100644 --- a/vendor/gems/discourse_emoji/vendor/assets/javascripts/discourse_emoji.js +++ b/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) ? - '' + m2 + '' : + '' + m2 + '' : m1; }); });