diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 21e8e1ef49b..f79e25d27ec 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -5,7 +5,6 @@ require_dependency 'age_words' class UserNotifications < ActionMailer::Base helper :application default charset: 'UTF-8' - layout 'user_notifications' include Email::BuildEmailHelper @@ -291,7 +290,6 @@ class UserNotifications < ActionMailer::Base else html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render( template: 'email/notification', - layout: 'layouts/user_notifications', format: :html, locals: { context_posts: context_posts, post: post, diff --git a/app/views/layouts/user_notifications.html.erb b/app/views/layouts/user_notifications.html.erb deleted file mode 100644 index 9202d99d699..00000000000 --- a/app/views/layouts/user_notifications.html.erb +++ /dev/null @@ -1,42 +0,0 @@ - - - - - - - - - - - - -
-
- - - - - -
- - <%= yield %> - -
- - -
-
- - \ No newline at end of file diff --git a/lib/email/styles.rb b/lib/email/styles.rb index 25d6a64141d..67447a10a00 100644 --- a/lib/email/styles.rb +++ b/lib/email/styles.rb @@ -9,7 +9,7 @@ module Email def initialize(html, opts=nil) @html = html @opts = opts || {} - @doc = Nokogiri::HTML(@html) + @fragment = Nokogiri::HTML.fragment(@html) end def self.register_plugin_style(&block) @@ -30,7 +30,7 @@ module Email uri = URI(Discourse.base_url) # images - @doc.css('img').each do |img| + @fragment.css('img').each do |img| next if img['class'] == 'site-logo' if img['class'] == "emoji" || img['src'] =~ /plugins\/emoji/ @@ -56,16 +56,15 @@ module Email end # add max-width to big images - big_images = @doc.css('img[width="auto"][height="auto"]') - - @doc.css('aside.onebox img') - - @doc.css('img.site-logo, img.emoji') + big_images = @fragment.css('img[width="auto"][height="auto"]') - + @fragment.css('aside.onebox img') - + @fragment.css('img.site-logo, img.emoji') big_images.each do |img| - add_styles(img, 'width: 100%; height: auto; max-width: 600px;') if img['style'] !~ /max-width/ - add_attributes(img, width: '600') + add_styles(img, 'max-width: 100%;') if img['style'] !~ /max-width/ end # attachments - @doc.css('a.attachment').each do |a| + @fragment.css('a.attachment').each do |a| # ensure all urls are absolute if a['href'] =~ /^\/[^\/]/ a['href'] = "#{Discourse.base_url}#{a['href']}" @@ -91,10 +90,6 @@ module Email style('.rtl', 'direction: rtl;') style('td.body', 'padding-top:5px;', colspan: "2") style('.whisper td.body', 'font-style: italic; color: #9c9c9c;') - style('.wrapper', 'width:100%;table-layout:fixed;-webkit-text-size-adjust:100%;-ms-text-size-adjust:100%;') - style('.webkit', 'max-width:600px;') - style('.outer', 'Margin: 0 auto; width: 100%; max-width: 600px;') - style('.container', 'padding: 10px;') correct_first_body_margin correct_footer_style reset_tables @@ -122,12 +117,12 @@ module Email style('aside.clearfix', "clear: both") # Finally, convert all `aside` tags to `div`s - @doc.css('aside, article, header').each do |n| + @fragment.css('aside, article, header').each do |n| n.name = "div" end # iframes can't go in emails, so replace them with clickable links - @doc.css('iframe').each do |i| + @fragment.css('iframe').each do |i| begin src_uri = URI(i['src']) @@ -142,7 +137,6 @@ module Email end def format_html - style('body', 'Margin: 0;padding:0;min-width:100%;background-color:#ffffff;') style('h3', 'margin: 15px 0 20px 0;') style('hr', 'background-color: #ddd; height: 1px; border: 1px;') style('a', 'text-decoration: none; font-weight: bold; color: #006699;') @@ -163,21 +157,20 @@ module Email # this method is reserved for styles specific to plugin def plugin_styles - @@plugin_callbacks.each { |block| block.call(@doc, @opts) } + @@plugin_callbacks.each { |block| block.call(@fragment, @opts) } end def to_html - return "" unless has_body? strip_classes_and_ids replace_relative_urls - @doc.to_html.tap do |result| + @fragment.to_html.tap do |result| result.gsub!(/\[email-indent\]/, "
") result.gsub!(/\[\/email-indent\]/, "
") end end def strip_avatars_and_emojis - @doc.search('img').each do |img| + @fragment.search('img').each do |img| if img['src'] =~ /_avatar/ img.parent['style'] = "vertical-align: top;" if img.parent.name == 'td' img.remove @@ -189,7 +182,7 @@ module Email end end - @doc.to_s + @fragment.to_s end private @@ -199,7 +192,7 @@ module Email host = forum_uri.host scheme = forum_uri.scheme - @doc.css('[href]').each do |element| + @fragment.css('[href]').each do |element| href = element['href'] if href =~ /^\/\/#{host}/ element['href'] = "#{scheme}:#{href}" @@ -208,14 +201,14 @@ module Email end def correct_first_body_margin - @doc.css('.body p').each do |element| + @fragment.css('.body p').each do |element| element['style'] = "margin-top:0; border: 0;" end end def correct_footer_style footernum = 0 - @doc.css('.footer').each do |element| + @fragment.css('.footer').each do |element| element['style'] = "color:#666;" linknum = 0 element.css('a').each do |inner| @@ -232,7 +225,7 @@ module Email end def strip_classes_and_ids - @doc.css('*').each do |element| + @fragment.css('*').each do |element| element.delete('class') element.delete('id') end @@ -242,20 +235,12 @@ module Email style('table', nil, cellspacing: '0', cellpadding: '0', border: '0') end - def add_attributes(element, attribs) - attribs.each do |k, v| - element[k] = v - end - end - - def has_body? - @doc.at_css('body') - end - def style(selector, style, attribs = {}) - @doc.css(selector).each do |element| + @fragment.css(selector).each do |element| add_styles(element, style) if style - add_attributes(element, attribs) + attribs.each do |k,v| + element[k] = v + end end end end diff --git a/spec/components/email/styles_spec.rb b/spec/components/email/styles_spec.rb index e471c310fd1..1f8bb953bf1 100644 --- a/spec/components/email/styles_spec.rb +++ b/spec/components/email/styles_spec.rb @@ -3,25 +3,17 @@ require 'email' describe Email::Styles do - def basic_doc(html) + def basic_fragment(html) styler = Email::Styles.new(html) styler.format_basic - Nokogiri::HTML(styler.to_html) + Nokogiri::HTML.fragment(styler.to_html) end - def html_doc(html) + def html_fragment(html) styler = Email::Styles.new(html) styler.format_basic styler.format_html - Nokogiri::HTML(styler.to_html) - end - - def notification_doc(html) - styler = Email::Styles.new(html) - styler.format_basic - styler.format_notification - styler.format_html - Nokogiri::HTML(styler.to_html) + Nokogiri::HTML.fragment(styler.to_html) end context "basic formatter" do @@ -32,30 +24,26 @@ describe Email::Styles do expect(style.to_html).to be_blank end - it "adds a max-width to big images" do - doc = basic_doc("") - expect(doc.at("img")["style"]).to match("max-width") - end - - it "doesn't add a maz-width to small images" do - doc = basic_doc("") - expect(doc.at("img")["style"]).not_to match("max-width") + # Pending due to email effort @coding-horror made in d2fb2bc4c + skip "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 - doc = basic_doc("") - expect(doc.at("img")["width"]).to eq("20") - expect(doc.at("img")["height"]).to eq("20") + frag = basic_fragment("") + expect(frag.at("img")["width"]).to eq("20") + expect(frag.at("img")["height"]).to eq("20") end it "converts relative paths to absolute paths" do - doc = basic_doc("") - expect(doc.at("img")["src"]).to eq("#{Discourse.base_url}/some-image.png") + frag = basic_fragment("") + expect(frag.at("img")["src"]).to eq("#{Discourse.base_url}/some-image.png") end it "strips classes and ids" do - doc = basic_doc("
") - expect(doc.to_html).to match(/
<\/div><\/div>/) + frag = basic_fragment("
") + expect(frag.to_html).to eq("
") end end @@ -68,75 +56,51 @@ describe Email::Styles do end it "attaches a style to h3 tags" do - doc = html_doc("

hello

") - expect(doc.at('h3')['style']).to be_present + frag = html_fragment("

hello

") + expect(frag.at('h3')['style']).to be_present end it "attaches a style to hr tags" do - doc = html_doc("hello
") - expect(doc.at('hr')['style']).to be_present + frag = html_fragment("hello
") + expect(frag.at('hr')['style']).to be_present end it "attaches a style to a tags" do - doc = html_doc("wat") - expect(doc.at('a')['style']).to be_present + frag = html_fragment("wat") + expect(frag.at('a')['style']).to be_present end it "attaches a style to a tags" do - doc = html_doc("wat") - expect(doc.at('a')['style']).to be_present + frag = html_fragment("wat") + expect(frag.at('a')['style']).to be_present end it "attaches a style to ul and li tags" do - doc = html_doc("") - expect(doc.at('ul')['style']).to be_present - expect(doc.at('li')['style']).to be_present + frag = html_fragment("") + expect(frag.at('ul')['style']).to be_present + expect(frag.at('li')['style']).to be_present end it "converts iframes to links" do iframe_url = "http://www.youtube.com/embed/7twifrxOTQY?feature=oembed&wmode=opaque" - doc = html_doc("") - expect(doc.at('iframe')).to be_blank - expect(doc.at('a')).to be_present - expect(doc.at('a')['href']).to eq(iframe_url) + frag = html_fragment("") + expect(frag.at('iframe')).to be_blank + expect(frag.at('a')).to be_present + expect(frag.at('a')['href']).to eq(iframe_url) end it "won't allow non URLs in iframe src, strips them with no link" do iframe_url = "alert('xss hole')" - doc = html_doc("") - expect(doc.at('iframe')).to be_blank - expect(doc.at('a')).to be_blank - end - end - - context "format notifications" do - it "adds both styles and attributes" do - doc = notification_doc("
hello
") - expect(doc.at('td')['style']).to eq('padding-top:5px;') - expect(doc.at('td')['colspan']).to eq('2') - end - - it "adds attributes when no styles are present" do - doc = notification_doc("
") - expect(doc.at('img')['width']).to eq('45') - end - - it "adds correct styles to the wrapper" do - doc = notification_doc('
') - expect(doc.at('center')['style']).to eq('width:100%;table-layout:fixed;-webkit-text-size-adjust:100%;-ms-text-size-adjust:100%;') - end - - it "doesn't override inline attributes" do - doc = notification_doc('
test
') - expect(doc.at('table')['align']).to eq('center') - expect(doc.at('table')['width']).to eq('600') + frag = html_fragment("") + expect(frag.at('iframe')).to be_blank + expect(frag.at('a')).to be_blank end end context "rewriting protocol relative URLs to the forum" do it "doesn't rewrite a url to another site" do - doc = html_doc('hello') - expect(doc.at('a')['href']).to eq("//youtube.com/discourse") + frag = html_fragment('hello') + expect(frag.at('a')['href']).to eq("//youtube.com/discourse") end context "without https" do @@ -145,18 +109,18 @@ describe Email::Styles do end it "rewrites the href to have http" do - doc = html_doc('hello') - expect(doc.at('a')['href']).to eq("http://test.localhost/discourse") + frag = html_fragment('hello') + expect(frag.at('a')['href']).to eq("http://test.localhost/discourse") end it "rewrites the href for attachment files to have http" do - doc = html_doc('attachment_file.txt') - expect(doc.at('a')['href']).to eq("http://try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt") + frag = html_fragment('attachment_file.txt') + expect(frag.at('a')['href']).to eq("http://try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt") end it "rewrites the src to have http" do - doc = html_doc('') - expect(doc.at('img')['src']).to eq("http://test.localhost/blah.jpg") + frag = html_fragment('') + expect(frag.at('img')['src']).to eq("http://test.localhost/blah.jpg") end end @@ -166,18 +130,18 @@ describe Email::Styles do end it "rewrites the forum URL to have https" do - doc = html_doc('hello') - expect(doc.at('a')['href']).to eq("https://test.localhost/discourse") + frag = html_fragment('hello') + expect(frag.at('a')['href']).to eq("https://test.localhost/discourse") end it "rewrites the href for attachment files to have https" do - doc = html_doc('attachment_file.txt') - expect(doc.at('a')['href']).to eq("https://try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt") + frag = html_fragment('attachment_file.txt') + expect(frag.at('a')['href']).to eq("https://try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt") end it "rewrites the src to have https" do - doc = html_doc('') - expect(doc.at('img')['src']).to eq("https://test.localhost/blah.jpg") + frag = html_fragment('') + expect(frag.at('img')['src']).to eq("https://test.localhost/blah.jpg") end end @@ -188,15 +152,16 @@ describe Email::Styles do emoji = "" style = Email::Styles.new(emoji) style.strip_avatars_and_emojis - expect(style.to_html).to match_html("#{emoji}") + expect(style.to_html).to match_html(emoji) end it "works for lonesome emoji with title" do emoji = "" style = Email::Styles.new(emoji) style.strip_avatars_and_emojis - expect(style.to_html).to match_html("cry_cry") + expect(style.to_html).to match_html("cry_cry") end end + end diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb index 7b56e2bb7ef..4c250f18e87 100644 --- a/spec/mailers/user_notifications_spec.rb +++ b/spec/mailers/user_notifications_spec.rb @@ -35,10 +35,6 @@ describe UserNotifications do expect(subject.body).to be_present end - it "does not use the layout" do - expect(subject.html_part.to_s.scan(//).count).to eq(0) - end - end describe ".forgot_password" do @@ -52,10 +48,6 @@ describe UserNotifications do expect(subject.body).to be_present end - it "does not use the layout" do - expect(subject.html_part.to_s.scan(//).count).to eq(0) - end - end describe '.digest' do @@ -85,10 +77,6 @@ describe UserNotifications do expect(subject.text_part.body.to_s).to be_present end - it "uses the layout" do - expect(subject.html_part.to_s.scan(//).count).to eq(1) - end - it "includes email_prefix in email subject instead of site title" do SiteSetting.email_prefix = "Try Discourse" SiteSetting.title = "Discourse Meta" @@ -116,8 +104,6 @@ describe UserNotifications do notification_type: notification.notification_type, notification_data_hash: notification.data_hash ) - # meta tag should be present from the layout - expect(mail.html_part.to_s.scan(//).count).to eq(1) # from should include full user name expect(mail[:from].display_names).to eql(['John Doe']) @@ -170,9 +156,6 @@ describe UserNotifications do notification_data_hash: notification.data_hash ) - # meta tag should be present from the layout - expect(mail.html_part.to_s.scan(//).count).to eq(1) - # from should not include full user name if "show user full names" is disabled expect(mail[:from].display_names).to_not eql(['John Doe']) @@ -210,9 +193,6 @@ describe UserNotifications do notification_data_hash: notification.data_hash ) - # meta tag should be present from the layout - expect(mail.html_part.to_s.scan(//).count).to eq(1) - # from should include username if full user name is not provided expect(mail[:from].display_names).to eql(['john'])