diff --git a/CHANGELOG.md b/CHANGELOG.md index f1c1690..ecd37c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## HEAD - Add support for rails' `relative_url_root` config +- Fix link tag removal under Hpricot ## v1.9.2 diff --git a/lib/premailer/rails/css_helper.rb b/lib/premailer/rails/css_helper.rb index 1e1929e..0f6741f 100644 --- a/lib/premailer/rails/css_helper.rb +++ b/lib/premailer/rails/css_helper.rb @@ -27,7 +27,11 @@ def css_for_url(url) def css_urls_in_doc(doc) doc.search('link[@rel="stylesheet"]:not([@data-premailer="ignore"])').map do |link| - link.remove + if link.respond_to?(:remove) + link.remove + else + link.parent.children.delete(link) + end link.attributes['href'].to_s end end diff --git a/spec/integration/css_helper_spec.rb b/spec/integration/css_helper_spec.rb index 67b8f9a..377b96a 100644 --- a/spec/integration/css_helper_spec.rb +++ b/spec/integration/css_helper_spec.rb @@ -1,180 +1,186 @@ require 'spec_helper' describe Premailer::Rails::CSSHelper do - # Reset the CSS cache: - after do - Premailer::Rails::CSSLoaders::CacheLoader.clear! - end - - def css_for_url(path) - Premailer::Rails::CSSHelper.css_for_url(path) - end + [ :Nokogiri, :Hpricot ].each do |adapter| + next if adapter == :Hpricot and RUBY_PLATFORM == 'java' - def css_for_doc(doc) - Premailer::Rails::CSSHelper.css_for_doc(doc) - end + context "when adapter is #{adapter}" do + # Reset the CSS cache: + after do + Premailer::Rails::CSSLoaders::CacheLoader.clear! + end - def expect_file(path, content='file content') - allow(File).to receive(:exist?).with(path).and_return(true) - expect(File).to receive(:read).with(path).and_return(content) - end + def css_for_url(path) + Premailer::Rails::CSSHelper.css_for_url(path) + end - describe '#css_for_doc' do - let(:html) { Fixtures::HTML.with_css_links(*files) } - let(:doc) { Nokogiri(html) } + def css_for_doc(doc) + Premailer::Rails::CSSHelper.css_for_doc(doc) + end - context 'when HTML contains linked CSS files' do - let(:files) { %w[ stylesheets/base.css stylesheets/font.css ] } + def expect_file(path, content='file content') + allow(File).to receive(:exist?).with(path).and_return(true) + expect(File).to receive(:read).with(path).and_return(content) + end - it 'returns the content of both files concatenated' do - allow(Premailer::Rails::CSSHelper).to \ - receive(:css_for_url) - .with('http://example.com/stylesheets/base.css') - .and_return('content of base.css') - allow(Premailer::Rails::CSSHelper).to \ - receive(:css_for_url) - .with('http://example.com/stylesheets/font.css') - .and_return('content of font.css') + describe '#css_for_doc' do + let(:html) { Fixtures::HTML.with_css_links(*files) } + let(:doc) { send(adapter, html) } - expect(css_for_doc(doc)).to eq("content of base.css\ncontent of font.css") - end - end + context 'when HTML contains linked CSS files' do + let(:files) { %w[ stylesheets/base.css stylesheets/font.css ] } - context 'when HTML contains ignored links' do - let(:files) { ['ignore.css', 'data-premailer' => 'ignore'] } + it 'returns the content of both files concatenated' do + allow(Premailer::Rails::CSSHelper).to \ + receive(:css_for_url) + .with('http://example.com/stylesheets/base.css') + .and_return('content of base.css') + allow(Premailer::Rails::CSSHelper).to \ + receive(:css_for_url) + .with('http://example.com/stylesheets/font.css') + .and_return('content of font.css') - it 'ignores links' do - expect(Premailer::Rails::CSSHelper).to_not receive(:css_for_url) - css_for_doc(doc) - end - end - end + expect(css_for_doc(doc)).to eq("content of base.css\ncontent of font.css") + end + end - describe '#css_for_url' do - context 'when path is a url' do - it 'loads the CSS at the local path' do - expect_file('public/stylesheets/base.css') + context 'when HTML contains ignored links' do + let(:files) { ['ignore.css', 'data-premailer' => 'ignore'] } - css_for_url('http://example.com/stylesheets/base.css?test') + it 'ignores links' do + expect(Premailer::Rails::CSSHelper).to_not receive(:css_for_url) + css_for_doc(doc) + end + end end - end - context 'when path is a relative url' do - it 'loads the CSS at the local path' do - expect_file('public/stylesheets/base.css') - css_for_url('/stylesheets/base.css?test') - end - end + describe '#css_for_url' do + context 'when path is a url' do + it 'loads the CSS at the local path' do + expect_file('public/stylesheets/base.css') - context 'when file is cached' do - it 'returns the cached value' do - Premailer::Rails::CSSLoaders::CacheLoader.store( - 'http://example.com/stylesheets/base.css', - 'content of base.css' - ) + css_for_url('http://example.com/stylesheets/base.css?test') + end + end - expect(css_for_url('http://example.com/stylesheets/base.css')).to \ - eq('content of base.css') - end - end + context 'when path is a relative url' do + it 'loads the CSS at the local path' do + expect_file('public/stylesheets/base.css') + css_for_url('/stylesheets/base.css?test') + end + end - context 'when in development mode' do - it 'does not return cached values' do - Premailer::Rails::CSSLoaders::CacheLoader.store( - 'http://example.com/stylesheets/base.css', - 'cached content of base.css' - ) - content = 'new content of base.css' - expect_file('public/stylesheets/base.css', content) - allow(Rails.env).to receive(:development?).and_return(true) - - expect(css_for_url('http://example.com/stylesheets/base.css')).to eq(content) - end - end + context 'when file is cached' do + it 'returns the cached value' do + Premailer::Rails::CSSLoaders::CacheLoader.store( + 'http://example.com/stylesheets/base.css', + 'content of base.css' + ) - context 'when Rails asset pipeline is used' do - before do - allow(Rails.configuration) - .to receive(:assets).and_return(double(prefix: '/assets')) - allow(Rails.configuration) - .to receive(:relative_url_root).and_return(nil) - end + expect(css_for_url('http://example.com/stylesheets/base.css')).to \ + eq('content of base.css') + end + end - context 'and a precompiled file exists' do - it 'returns that file' do - path = '/assets/email-digest.css' - content = 'read from file' - expect_file("public#{path}", content) - expect(css_for_url(path)).to eq(content) + context 'when in development mode' do + it 'does not return cached values' do + Premailer::Rails::CSSLoaders::CacheLoader.store( + 'http://example.com/stylesheets/base.css', + 'cached content of base.css' + ) + content = 'new content of base.css' + expect_file('public/stylesheets/base.css', content) + allow(Rails.env).to receive(:development?).and_return(true) + + expect(css_for_url('http://example.com/stylesheets/base.css')).to eq(content) + end end - end - it 'returns the content of the file compiled by Rails' do - expect(Rails.application.assets).to \ - receive(:find_asset) - .with('base.css') - .and_return(double(to_s: 'content of base.css')) + context 'when Rails asset pipeline is used' do + before do + allow(Rails.configuration) + .to receive(:assets).and_return(double(prefix: '/assets')) + allow(Rails.configuration) + .to receive(:relative_url_root).and_return(nil) + end - expect(css_for_url('http://example.com/assets/base.css')).to \ - eq('content of base.css') - end + context 'and a precompiled file exists' do + it 'returns that file' do + path = '/assets/email-digest.css' + content = 'read from file' + expect_file("public#{path}", content) + expect(css_for_url(path)).to eq(content) + end + end - it 'returns same file when path contains file fingerprint' do - expect(Rails.application.assets).to \ - receive(:find_asset) - .with('base.css') - .and_return(double(to_s: 'content of base.css')) + it 'returns the content of the file compiled by Rails' do + expect(Rails.application.assets).to \ + receive(:find_asset) + .with('base.css') + .and_return(double(to_s: 'content of base.css')) - expect(css_for_url( - 'http://example.com/assets/base-089e35bd5d84297b8d31ad552e433275.css' - )).to eq('content of base.css') - end + expect(css_for_url('http://example.com/assets/base.css')).to \ + eq('content of base.css') + end - context 'when asset can not be found' do - let(:response) { 'content of base.css' } - let(:path) { '/assets/base-089e35bd5d84297b8d31ad552e433275.css' } - let(:url) { "http://assets.example.com#{path}" } - let(:asset_host) { 'http://assets.example.com' } + it 'returns same file when path contains file fingerprint' do + expect(Rails.application.assets).to \ + receive(:find_asset) + .with('base.css') + .and_return(double(to_s: 'content of base.css')) - before do - allow(Rails.application.assets).to \ - receive(:find_asset).and_return(nil) + expect(css_for_url( + 'http://example.com/assets/base-089e35bd5d84297b8d31ad552e433275.css' + )).to eq('content of base.css') + end - config = double(asset_host: asset_host) - allow(Rails.configuration).to \ - receive(:action_controller).and_return(config) + context 'when asset can not be found' do + let(:response) { 'content of base.css' } + let(:path) { '/assets/base-089e35bd5d84297b8d31ad552e433275.css' } + let(:url) { "http://assets.example.com#{path}" } + let(:asset_host) { 'http://assets.example.com' } - uri_satisfaction = satisfy { |uri| uri.to_s == url } - allow(Net::HTTP).to \ - receive(:get).with(uri_satisfaction).and_return(response) - end + before do + allow(Rails.application.assets).to \ + receive(:find_asset).and_return(nil) - it 'requests the file' do - expect(css_for_url(url)).to eq('content of base.css') - end + config = double(asset_host: asset_host) + allow(Rails.configuration).to \ + receive(:action_controller).and_return(config) - context 'when file url does not include the host' do - it 'requests the file using the asset host as host' do - expect(css_for_url(path)).to eq('content of base.css') - end + uri_satisfaction = satisfy { |uri| uri.to_s == url } + allow(Net::HTTP).to \ + receive(:get).with(uri_satisfaction).and_return(response) + end - context 'and the asset host uses protocol relative scheme' do - let(:asset_host) { '//assets.example.com' } + it 'requests the file' do + expect(css_for_url(url)).to eq('content of base.css') + end - it 'requests the file using http as the scheme' do - expect(css_for_url(path)).to eq('content of base.css') + context 'when file url does not include the host' do + it 'requests the file using the asset host as host' do + expect(css_for_url(path)).to eq('content of base.css') + end + + context 'and the asset host uses protocol relative scheme' do + let(:asset_host) { '//assets.example.com' } + + it 'requests the file using http as the scheme' do + expect(css_for_url(path)).to eq('content of base.css') + end + end end end end - end - end - context 'when static stylesheets are used' do - it 'returns the content of the static file' do - content = 'content of base.css' - expect_file('public/stylesheets/base.css', content) - loaded_content = css_for_url('http://example.com/stylesheets/base.css') - expect(loaded_content).to eq(content) + context 'when static stylesheets are used' do + it 'returns the content of the static file' do + content = 'content of base.css' + expect_file('public/stylesheets/base.css', content) + loaded_content = css_for_url('http://example.com/stylesheets/base.css') + expect(loaded_content).to eq(content) + end + end end end end