codeninjasllc/discourse
1
0
Fork 0
mirror of https://github.com/codeninjasllc/discourse.git synced 2024-11-27 17:46:05 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

Merge pull request #3984 from discourse/revert-3969-email-layout

Browse source 
Revert "FEATURE (WIP): add max-width and center email notifications"
This commit is contained in:
Régis Hanol 2016-01-29 11:14:27 +01:00
commit 32ef138dd7
5 changed files with 71 additions and 185 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/mailers/user_notifications.rb
View file

@ -5,7 +5,6 @@ require_dependency 'age_words'
class UserNotifications < ActionMailer::Base class UserNotifications < ActionMailer::Base
helper :application helper :application
default charset: 'UTF-8' default charset: 'UTF-8'
layout 'user_notifications'
include Email::BuildEmailHelper include Email::BuildEmailHelper
@ -291,7 +290,6 @@ class UserNotifications < ActionMailer::Base
else else
html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render( html = UserNotificationRenderer.new(Rails.configuration.paths["app/views"]).render(
template: 'email/notification', template: 'email/notification',
layout: 'layouts/user_notifications',
format: :html, format: :html,
locals: { context_posts: context_posts, locals: { context_posts: context_posts,
post: post, post: post,

42
app/views/layouts/user_notifications.html.erb
View file

@ -1,42 +0,0 @@
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<!--[if !mso]><!-->
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<!--<![endif]-->
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title></title>
<!--[if (gte mso 9)|(IE)]>
<style type="text/css">
table {border-collapse: collapse;}
</style>
<![endif]-->
</head>
<body>
<center class="wrapper">
<div class="webkit">
<!--[if (gte mso 9)|(IE)]>
<table width="600" align="center">
<tr>
<td>
<![endif]-->
<table class="outer" align="center">
<tr>
<td class="container">
<%= yield %>
</td>
</tr>
</table>
<!--[if (gte mso 9)|(IE)]>
</td>
</tr>
</table>
<![endif]-->
</div>
</center>
</body>
</html>

57
lib/email/styles.rb
View file

@ -9,7 +9,7 @@ module Email
def initialize(html, opts=nil) def initialize(html, opts=nil)
@html = html @html = html
@opts = opts || {} @opts = opts || {}
@doc = Nokogiri::HTML(@html) @fragment = Nokogiri::HTML.fragment(@html)
end end
def self.register_plugin_style(&block) def self.register_plugin_style(&block)
@ -30,7 +30,7 @@ module Email
uri = URI(Discourse.base_url) uri = URI(Discourse.base_url)
# images # images
@doc.css('img').each do |img| @fragment.css('img').each do |img|
next if img['class'] == 'site-logo' next if img['class'] == 'site-logo'
if img['class'] == "emoji" || img['src'] =~ /plugins\/emoji/ if img['class'] == "emoji" || img['src'] =~ /plugins\/emoji/
@ -56,16 +56,15 @@ module Email
end end
# add max-width to big images # add max-width to big images
big_images = @doc.css('img[width="auto"][height="auto"]') - big_images = @fragment.css('img[width="auto"][height="auto"]') -
@doc.css('aside.onebox img') - @fragment.css('aside.onebox img') -
@doc.css('img.site-logo, img.emoji') @fragment.css('img.site-logo, img.emoji')
big_images.each do |img| big_images.each do |img|
add_styles(img, 'width: 100%; height: auto; max-width: 600px;') if img['style'] !~ /max-width/ add_styles(img, 'max-width: 100%;') if img['style'] !~ /max-width/
add_attributes(img, width: '600')
end end
# attachments # attachments
@doc.css('a.attachment').each do |a| @fragment.css('a.attachment').each do |a|
# ensure all urls are absolute # ensure all urls are absolute
if a['href'] =~ /^\/[^\/]/ if a['href'] =~ /^\/[^\/]/
a['href'] = "#{Discourse.base_url}#{a['href']}" a['href'] = "#{Discourse.base_url}#{a['href']}"
@ -91,10 +90,6 @@ module Email
style('.rtl', 'direction: rtl;') style('.rtl', 'direction: rtl;')
style('td.body', 'padding-top:5px;', colspan: "2") style('td.body', 'padding-top:5px;', colspan: "2")
style('.whisper td.body', 'font-style: italic; color: #9c9c9c;') 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_first_body_margin
correct_footer_style correct_footer_style
reset_tables reset_tables
@ -122,12 +117,12 @@ module Email
style('aside.clearfix', "clear: both") style('aside.clearfix', "clear: both")
# Finally, convert all `aside` tags to `div`s # 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" n.name = "div"
end end
# iframes can't go in emails, so replace them with clickable links # 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 begin
src_uri = URI(i['src']) src_uri = URI(i['src'])
@ -142,7 +137,6 @@ module Email
end end
def format_html def format_html
style('body', 'Margin: 0;padding:0;min-width:100%;background-color:#ffffff;')
style('h3', 'margin: 15px 0 20px 0;') style('h3', 'margin: 15px 0 20px 0;')
style('hr', 'background-color: #ddd; height: 1px; border: 1px;') style('hr', 'background-color: #ddd; height: 1px; border: 1px;')
style('a', 'text-decoration: none; font-weight: bold; color: #006699;') 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 # this method is reserved for styles specific to plugin
def plugin_styles def plugin_styles
@@plugin_callbacks.each { |block| block.call(@doc, @opts) } @@plugin_callbacks.each { |block| block.call(@fragment, @opts) }
end end
def to_html def to_html
return "" unless has_body?
strip_classes_and_ids strip_classes_and_ids
replace_relative_urls replace_relative_urls
@doc.to_html.tap do |result| @fragment.to_html.tap do |result|
result.gsub!(/\[email-indent\]/, "<div style='margin-left: 15px'>") result.gsub!(/\[email-indent\]/, "<div style='margin-left: 15px'>")
result.gsub!(/\[\/email-indent\]/, "</div>") result.gsub!(/\[\/email-indent\]/, "</div>")
end end
end end
def strip_avatars_and_emojis def strip_avatars_and_emojis
@doc.search('img').each do |img| @fragment.search('img').each do |img|
if img['src'] =~ /_avatar/ if img['src'] =~ /_avatar/
img.parent['style'] = "vertical-align: top;" if img.parent.name == 'td' img.parent['style'] = "vertical-align: top;" if img.parent.name == 'td'
img.remove img.remove
@ -189,7 +182,7 @@ module Email
end end
end end
@doc.to_s @fragment.to_s
end end
private private
@ -199,7 +192,7 @@ module Email
host = forum_uri.host host = forum_uri.host
scheme = forum_uri.scheme scheme = forum_uri.scheme
@doc.css('[href]').each do |element| @fragment.css('[href]').each do |element|
href = element['href'] href = element['href']
if href =~ /^\/\/#{host}/ if href =~ /^\/\/#{host}/
element['href'] = "#{scheme}:#{href}" element['href'] = "#{scheme}:#{href}"
@ -208,14 +201,14 @@ module Email
end end
def correct_first_body_margin 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;" element['style'] = "margin-top:0; border: 0;"
end end
end end
def correct_footer_style def correct_footer_style
footernum = 0 footernum = 0
@doc.css('.footer').each do |element| @fragment.css('.footer').each do |element|
element['style'] = "color:#666;" element['style'] = "color:#666;"
linknum = 0 linknum = 0
element.css('a').each do |inner| element.css('a').each do |inner|
@ -232,7 +225,7 @@ module Email
end end
def strip_classes_and_ids def strip_classes_and_ids
@doc.css('*').each do |element| @fragment.css('*').each do |element|
element.delete('class') element.delete('class')
element.delete('id') element.delete('id')
end end
@ -242,20 +235,12 @@ module Email
style('table', nil, cellspacing: '0', cellpadding: '0', border: '0') style('table', nil, cellspacing: '0', cellpadding: '0', border: '0')
end 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 = {}) def style(selector, style, attribs = {})
@doc.css(selector).each do |element| @fragment.css(selector).each do |element|
add_styles(element, style) if style add_styles(element, style) if style
add_attributes(element, attribs) attribs.each do |k,v|
element[k] = v
end
end end
end end
end end

135
spec/components/email/styles_spec.rb
View file

@ -3,25 +3,17 @@ require 'email'
describe Email::Styles do describe Email::Styles do
def basic_doc(html) def basic_fragment(html)
styler = Email::Styles.new(html) styler = Email::Styles.new(html)
styler.format_basic styler.format_basic
Nokogiri::HTML(styler.to_html) Nokogiri::HTML.fragment(styler.to_html)
end end
def html_doc(html) def html_fragment(html)
styler = Email::Styles.new(html) styler = Email::Styles.new(html)
styler.format_basic styler.format_basic
styler.format_html styler.format_html
Nokogiri::HTML(styler.to_html) Nokogiri::HTML.fragment(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)
end end
context "basic formatter" do context "basic formatter" do
@ -32,30 +24,26 @@ describe Email::Styles do
expect(style.to_html).to be_blank expect(style.to_html).to be_blank
end end
it "adds a max-width to big images" do # Pending due to email effort @coding-horror made in d2fb2bc4c
doc = basic_doc("<img src='gigantic.jpg' width='1230' height='720'>") skip "adds a max-width to images" do
expect(doc.at("img")["style"]).to match("max-width") frag = basic_fragment("<img src='gigantic.jpg'>")
end expect(frag.at("img")["style"]).to match("max-width")
it "doesn't add a maz-width to small images" do
doc = basic_doc("<img src='gigantic.jpg' width='120' height='80'>")
expect(doc.at("img")["style"]).not_to match("max-width")
end end
it "adds a width and height to images with an emoji path" do it "adds a width and height to images with an emoji path" do
doc = basic_doc("<img src='/images/emoji/fish.png' class='emoji'>") frag = basic_fragment("<img src='/images/emoji/fish.png' class='emoji'>")
expect(doc.at("img")["width"]).to eq("20") expect(frag.at("img")["width"]).to eq("20")
expect(doc.at("img")["height"]).to eq("20") expect(frag.at("img")["height"]).to eq("20")
end end
it "converts relative paths to absolute paths" do it "converts relative paths to absolute paths" do
doc = basic_doc("<img src='/some-image.png'>") frag = basic_fragment("<img src='/some-image.png'>")
expect(doc.at("img")["src"]).to eq("#{Discourse.base_url}/some-image.png") expect(frag.at("img")["src"]).to eq("#{Discourse.base_url}/some-image.png")
end end
it "strips classes and ids" do it "strips classes and ids" do
doc = basic_doc("<div class='foo' id='bar'><div class='foo' id='bar'></div></div>") frag = basic_fragment("<div class='foo' id='bar'><div class='foo' id='bar'></div></div>")
expect(doc.to_html).to match(/<div><div><\/div><\/div>/) expect(frag.to_html).to eq("<div><div></div></div>")
end end
end end
@ -68,75 +56,51 @@ describe Email::Styles do
end end
it "attaches a style to h3 tags" do it "attaches a style to h3 tags" do
doc = html_doc("<h3>hello</h3>") frag = html_fragment("<h3>hello</h3>")
expect(doc.at('h3')['style']).to be_present expect(frag.at('h3')['style']).to be_present
end end
it "attaches a style to hr tags" do it "attaches a style to hr tags" do
doc = html_doc("hello<hr>") frag = html_fragment("hello<hr>")
expect(doc.at('hr')['style']).to be_present expect(frag.at('hr')['style']).to be_present
end end
it "attaches a style to a tags" do it "attaches a style to a tags" do
doc = html_doc("<a href>wat</a>") frag = html_fragment("<a href>wat</a>")
expect(doc.at('a')['style']).to be_present expect(frag.at('a')['style']).to be_present
end end
it "attaches a style to a tags" do it "attaches a style to a tags" do
doc = html_doc("<a href>wat</a>") frag = html_fragment("<a href>wat</a>")
expect(doc.at('a')['style']).to be_present expect(frag.at('a')['style']).to be_present
end end
it "attaches a style to ul and li tags" do it "attaches a style to ul and li tags" do
doc = html_doc("<ul><li>hello</li></ul>") frag = html_fragment("<ul><li>hello</li></ul>")
expect(doc.at('ul')['style']).to be_present expect(frag.at('ul')['style']).to be_present
expect(doc.at('li')['style']).to be_present expect(frag.at('li')['style']).to be_present
end end
it "converts iframes to links" do it "converts iframes to links" do
iframe_url = "http://www.youtube.com/embed/7twifrxOTQY?feature=oembed&wmode=opaque" iframe_url = "http://www.youtube.com/embed/7twifrxOTQY?feature=oembed&wmode=opaque"
doc = html_doc("<iframe src=\"#{iframe_url}\"></iframe>") frag = html_fragment("<iframe src=\"#{iframe_url}\"></iframe>")
expect(doc.at('iframe')).to be_blank expect(frag.at('iframe')).to be_blank
expect(doc.at('a')).to be_present expect(frag.at('a')).to be_present
expect(doc.at('a')['href']).to eq(iframe_url) expect(frag.at('a')['href']).to eq(iframe_url)
end end
it "won't allow non URLs in iframe src, strips them with no link" do it "won't allow non URLs in iframe src, strips them with no link" do
iframe_url = "alert('xss hole')" iframe_url = "alert('xss hole')"
doc = html_doc("<iframe src=\"#{iframe_url}\"></iframe>") frag = html_fragment("<iframe src=\"#{iframe_url}\"></iframe>")
expect(doc.at('iframe')).to be_blank expect(frag.at('iframe')).to be_blank
expect(doc.at('a')).to be_blank expect(frag.at('a')).to be_blank
end
end
context "format notifications" do
it "adds both styles and attributes" do
doc = notification_doc("<table><tr><td class='body'>hello</td></tr></table>")
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("<div class='user-avatar'><img src='/some-image.png'></div>")
expect(doc.at('img')['width']).to eq('45')
end
it "adds correct styles to the wrapper" do
doc = notification_doc('<center class="wrapper"></center>')
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('<table width="600" align="center"><tr><td>test</td></tr></table>')
expect(doc.at('table')['align']).to eq('center')
expect(doc.at('table')['width']).to eq('600')
end end
end end
context "rewriting protocol relative URLs to the forum" do context "rewriting protocol relative URLs to the forum" do
it "doesn't rewrite a url to another site" do it "doesn't rewrite a url to another site" do
doc = html_doc('<a href="//youtube.com/discourse">hello</a>') frag = html_fragment('<a href="//youtube.com/discourse">hello</a>')
expect(doc.at('a')['href']).to eq("//youtube.com/discourse") expect(frag.at('a')['href']).to eq("//youtube.com/discourse")
end end
context "without https" do context "without https" do
@ -145,18 +109,18 @@ describe Email::Styles do
end end
it "rewrites the href to have http" do it "rewrites the href to have http" do
doc = html_doc('<a href="//test.localhost/discourse">hello</a>') frag = html_fragment('<a href="//test.localhost/discourse">hello</a>')
expect(doc.at('a')['href']).to eq("http://test.localhost/discourse") expect(frag.at('a')['href']).to eq("http://test.localhost/discourse")
end end
it "rewrites the href for attachment files to have http" do it "rewrites the href for attachment files to have http" do
doc = html_doc('<a class="attachment" href="//try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt">attachment_file.txt</a>') frag = html_fragment('<a class="attachment" href="//try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt">attachment_file.txt</a>')
expect(doc.at('a')['href']).to eq("http://try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt") expect(frag.at('a')['href']).to eq("http://try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt")
end end
it "rewrites the src to have http" do it "rewrites the src to have http" do
doc = html_doc('<img src="//test.localhost/blah.jpg">') frag = html_fragment('<img src="//test.localhost/blah.jpg">')
expect(doc.at('img')['src']).to eq("http://test.localhost/blah.jpg") expect(frag.at('img')['src']).to eq("http://test.localhost/blah.jpg")
end end
end end
@ -166,18 +130,18 @@ describe Email::Styles do
end end
it "rewrites the forum URL to have https" do it "rewrites the forum URL to have https" do
doc = html_doc('<a href="//test.localhost/discourse">hello</a>') frag = html_fragment('<a href="//test.localhost/discourse">hello</a>')
expect(doc.at('a')['href']).to eq("https://test.localhost/discourse") expect(frag.at('a')['href']).to eq("https://test.localhost/discourse")
end end
it "rewrites the href for attachment files to have https" do it "rewrites the href for attachment files to have https" do
doc = html_doc('<a class="attachment" href="//try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt">attachment_file.txt</a>') frag = html_fragment('<a class="attachment" href="//try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt">attachment_file.txt</a>')
expect(doc.at('a')['href']).to eq("https://try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt") expect(frag.at('a')['href']).to eq("https://try-discourse.global.ssl.fastly.net/uploads/default/368/40b610b0aa90cfcf.txt")
end end
it "rewrites the src to have https" do it "rewrites the src to have https" do
doc = html_doc('<img src="//test.localhost/blah.jpg">') frag = html_fragment('<img src="//test.localhost/blah.jpg">')
expect(doc.at('img')['src']).to eq("https://test.localhost/blah.jpg") expect(frag.at('img')['src']).to eq("https://test.localhost/blah.jpg")
end end
end end
@ -188,15 +152,16 @@ describe Email::Styles do
emoji = "<img src='/images/emoji/emoji_one/crying_cat_face.png'>" emoji = "<img src='/images/emoji/emoji_one/crying_cat_face.png'>"
style = Email::Styles.new(emoji) style = Email::Styles.new(emoji)
style.strip_avatars_and_emojis style.strip_avatars_and_emojis
expect(style.to_html).to match_html("<!DOCTYPE html PUBLIC \"-//W3C//DTD HTML 4.0 Transitional//EN\" \"http://www.w3.org/TR/REC-html40/loose.dtd\"><html><body>#{emoji}</body></html>") expect(style.to_html).to match_html(emoji)
end end
it "works for lonesome emoji with title" do it "works for lonesome emoji with title" do
emoji = "<img title='cry_cry' src='/images/emoji/emoji_one/crying_cat_face.png'>" emoji = "<img title='cry_cry' src='/images/emoji/emoji_one/crying_cat_face.png'>"
style = Email::Styles.new(emoji) style = Email::Styles.new(emoji)
style.strip_avatars_and_emojis style.strip_avatars_and_emojis
expect(style.to_html).to match_html("<!DOCTYPE html PUBLIC \"-//W3C//DTD HTML 4.0 Transitional//EN\" \"http://www.w3.org/TR/REC-html40/loose.dtd\"><html><body>cry_cry</body></html>") expect(style.to_html).to match_html("cry_cry")
end end
end end
end end

20
spec/mailers/user_notifications_spec.rb
View file

@ -35,10 +35,6 @@ describe UserNotifications do
expect(subject.body).to be_present expect(subject.body).to be_present
end end
it "does not use the layout" do
expect(subject.html_part.to_s.scan(/<meta name="viewport" content="width=device-width, initial-scale=1.0">/).count).to eq(0)
end
end end
describe ".forgot_password" do describe ".forgot_password" do
@ -52,10 +48,6 @@ describe UserNotifications do
expect(subject.body).to be_present expect(subject.body).to be_present
end end
it "does not use the layout" do
expect(subject.html_part.to_s.scan(/<meta name="viewport" content="width=device-width, initial-scale=1.0">/).count).to eq(0)
end
end end
describe '.digest' do describe '.digest' do
@ -85,10 +77,6 @@ describe UserNotifications do
expect(subject.text_part.body.to_s).to be_present expect(subject.text_part.body.to_s).to be_present
end end
it "uses the layout" do
expect(subject.html_part.to_s.scan(/<meta name="viewport" content="width=device-width, initial-scale=1.0">/).count).to eq(1)
end
it "includes email_prefix in email subject instead of site title" do it "includes email_prefix in email subject instead of site title" do
SiteSetting.email_prefix = "Try Discourse" SiteSetting.email_prefix = "Try Discourse"
SiteSetting.title = "Discourse Meta" SiteSetting.title = "Discourse Meta"
@ -116,8 +104,6 @@ describe UserNotifications do
notification_type: notification.notification_type, notification_type: notification.notification_type,
notification_data_hash: notification.data_hash notification_data_hash: notification.data_hash
) )
# meta tag should be present from the layout
expect(mail.html_part.to_s.scan(/<meta name="viewport" content="width=device-width, initial-scale=1.0">/).count).to eq(1)
# from should include full user name # from should include full user name
expect(mail[:from].display_names).to eql(['John Doe']) expect(mail[:from].display_names).to eql(['John Doe'])
@ -170,9 +156,6 @@ describe UserNotifications do
notification_data_hash: notification.data_hash notification_data_hash: notification.data_hash
) )
# meta tag should be present from the layout
expect(mail.html_part.to_s.scan(/<meta name="viewport" content="width=device-width, initial-scale=1.0">/).count).to eq(1)
# from should not include full user name if "show user full names" is disabled # from should not include full user name if "show user full names" is disabled
expect(mail[:from].display_names).to_not eql(['John Doe']) expect(mail[:from].display_names).to_not eql(['John Doe'])
@ -210,9 +193,6 @@ describe UserNotifications do
notification_data_hash: notification.data_hash notification_data_hash: notification.data_hash
) )
# meta tag should be present from the layout
expect(mail.html_part.to_s.scan(/<meta name="viewport" content="width=device-width, initial-scale=1.0">/).count).to eq(1)
# from should include username if full user name is not provided # from should include username if full user name is not provided
expect(mail[:from].display_names).to eql(['john']) expect(mail[:from].display_names).to eql(['john'])