Replace URI with Addressable::URI, drop wrong sorting of attributes. …

…Drop last semicolon from attributes. Update tests.

.gitignore

@@ -9,3 +9,5 @@
 /pkg/
 /.bundle/
 /*.sublime-*
+/Gemfile.lock
+

.travis.yml

@@ -4,7 +4,6 @@ branches:
   only: master
 matrix:
   fast_finish: true
-before_install: rm Gemfile.lock
 language: ruby
 rvm:
   - 2.1.9

lib/premailer.rb

@@ -2,6 +2,7 @@
 require 'open-uri'
 require 'digest/md5'
 require 'cgi'
+require 'addressable'
 require 'css_parser'
 
 require 'premailer/adapter'

lib/premailer/adapter/nokogiri.rb

@@ -77,7 +77,7 @@ def to_inline_css
           if Premailer::RELATED_ATTRIBUTES.has_key?(el.name) && @options[:css_to_attributes]
             Premailer::RELATED_ATTRIBUTES[el.name].each do |css_att, html_att|
               if el[html_att].nil? and not merged[css_att].empty?
-                new_html_att = merged[css_att].gsub(/url\(['|"](.*)['|"]\)/, '\1').gsub(/;$|\s*!important/, '').strip
+                new_html_att = merged[css_att].gsub(/url\(['"](.*)['"]\)/, '\1').gsub(/;$|\s*!important/, '').strip
                 el[html_att] = css_att.end_with?('color') && @options[:rgb_to_hex_attributes] ? ensure_hex(new_html_att) : new_html_att
               end
               unless @options[:preserve_style_attribute]
@@ -91,10 +91,7 @@ def to_inline_css
           merged.create_shorthand! if @options[:create_shorthands]
 
           # write the inline STYLE attribute
-          # split by ';' but ignore those in brackets
-          attributes = Premailer.escape_string(merged.declarations_to_s).split(/;(?![^(]*\))/).map(&:strip)
-          attributes = attributes.map { |attr| [attr.split(':').first, attr] }.sort_by { |pair| pair.first }.map { |pair| pair[1] }
-          el['style'] = attributes.join('; ') + ";"
+          el['style'] = merged.declarations_to_s
         end
 
         doc = write_unmergable_css_rules(doc, @unmergable_rules)

lib/premailer/adapter/nokogiri_fast.rb

@@ -79,7 +79,7 @@ def to_inline_css
           if Premailer::RELATED_ATTRIBUTES.has_key?(el.name) && @options[:css_to_attributes]
             Premailer::RELATED_ATTRIBUTES[el.name].each do |css_att, html_att|
               if el[html_att].nil? and not merged[css_att].empty?
-                new_html_att = merged[css_att].gsub(/url\(['|"](.*)['|"]\)/, '\1').gsub(/;$|\s*!important/, '').strip
+                new_html_att = merged[css_att].gsub(/url\(['"](.*)['"]\)/, '\1').gsub(/;$|\s*!important/, '').strip
                 el[html_att] = css_att.end_with?('color') && @options[:rgb_to_hex_attributes] ? ensure_hex(new_html_att) : new_html_att
               end
               unless @options[:preserve_style_attribute]
@@ -93,10 +93,7 @@ def to_inline_css
           merged.create_shorthand! if @options[:create_shorthands]
 
           # write the inline STYLE attribute
-          # split by ';' but ignore those in brackets
-          attributes = Premailer.escape_string(merged.declarations_to_s).split(/;(?![^(]*\))/).map(&:strip)
-          attributes = attributes.map { |attr| [attr.split(':').first, attr] }.sort_by { |pair| pair.first }.map { |pair| pair[1] }
-          el['style'] = attributes.join('; ') + ";"
+          el['style'] = merged.declarations_to_s
         end
 
         doc = write_unmergable_css_rules(doc, @unmergable_rules)

lib/premailer/adapter/nokogumbo.rb

@@ -77,7 +77,7 @@ def to_inline_css
           if Premailer::RELATED_ATTRIBUTES.has_key?(el.name) && @options[:css_to_attributes]
             Premailer::RELATED_ATTRIBUTES[el.name].each do |css_att, html_att|
               if el[html_att].nil? and not merged[css_att].empty?
-                new_html_att = merged[css_att].gsub(/url\(['|"](.*)['|"]\)/, '\1').gsub(/;$|\s*!important/, '').strip
+                new_html_att = merged[css_att].gsub(/url\(['"](.*)['"]\)/, '\1').gsub(/;$|\s*!important/, '').strip
                 el[html_att] = css_att.end_with?('color') && @options[:rgb_to_hex_attributes] ? ensure_hex(new_html_att) : new_html_att
               end
               unless @options[:preserve_style_attribute]
@@ -91,9 +91,7 @@ def to_inline_css
           merged.create_shorthand! if @options[:create_shorthands]
 
           # write the inline STYLE attribute
-          attributes = Premailer.escape_string(merged.declarations_to_s).split(';').map(&:strip)
-          attributes = attributes.map { |attr| [attr.split(':').first, attr] }.sort_by { |pair| pair.first }.map { |pair| pair[1] }
-          el['style'] = attributes.join('; ') + ";"
+          el['style'] = merged.declarations_to_s
         end
 
         doc = write_unmergable_css_rules(doc, @unmergable_rules)

lib/premailer/premailer.rb

@@ -221,9 +221,9 @@ def initialize(html, options = {})
     @unmergable_rules = nil
 
     if @options[:base_url]
-      @base_url = URI.parse(@options.delete(:base_url))
+      @base_url = Addressable::URI.parse(@options.delete(:base_url))
     elsif not @is_local_file
-      @base_url = URI.parse(@html_file)
+      @base_url = Addressable::URI.parse(@html_file)
     end
 
     @css_parser = CssParser::Parser.new({
@@ -369,7 +369,7 @@ def append_query_string(doc, qs)
       next if href[0,1] =~ /[\#\{\[\<\%]/ # don't bother with anchors or special-looking links
 
       begin
-        href = URI.parse(href)
+        href = Addressable::URI.parse(href)
 
         if current_host and href.host != nil and href.host != current_host
           $stderr.puts "Skipping append_query_string for: #{href.to_s} because host is no good" if @options[:verbose]
@@ -389,7 +389,7 @@ def append_query_string(doc, qs)
         end
 
         el['href'] = href.to_s
-      rescue URI::Error => e
+      rescue Addressable::URI::InvalidURIError => e
         $stderr.puts "Skipping append_query_string for: #{href.to_s} (#{e.message})" if @options[:verbose]
         next
       end
@@ -415,7 +415,7 @@ def is_xhtml?
   #
   # Returns a document.
   def convert_inline_links(doc, base_uri) # :nodoc:
-    base_uri = URI.parse(base_uri) unless base_uri.kind_of?(URI)
+    base_uri = Addressable::URI.parse(base_uri) unless base_uri.kind_of?(Addressable::URI)
 
     append_qs = @options[:link_query_string] || ''
     escape_attrs = @options[:escape_url_attributes]
@@ -434,21 +434,21 @@ def convert_inline_links(doc, base_uri) # :nodoc:
 
         if tag.attributes[attribute].to_s =~ /^http/i
           begin
-            merged = URI.parse(tag.attributes[attribute])
+            merged = Addressable::URI.parse(tag.attributes[attribute])
           rescue; next; end
         else
           begin
             merged = Premailer.resolve_link(tag.attributes[attribute].to_s, base_uri)
           rescue
             begin
               next unless escape_attrs
-              merged = Premailer.resolve_link(URI.escape(tag.attributes[attribute].to_s), base_uri)
+              merged = Premailer.resolve_link(Addressable::URI.escape(tag.attributes[attribute].to_s), base_uri)
             rescue; end
           end
         end
 
         # make sure 'merged' is a URI
-        merged = URI.parse(merged.to_s) unless merged.kind_of?(URI)
+        merged = Addressable::URI.parse(merged.to_s) unless merged.kind_of?(Addressable::URI)
         tag[attribute] = merged.to_s
       end # end of each tag
     end # end of each attrs
@@ -476,12 +476,12 @@ def self.resolve_link(path, base_path) # :nodoc:
     if path =~ /\A(?:(https?|ftp|file):)\/\//i
       resolved = path
       Premailer.canonicalize(resolved)
-    elsif base_path.kind_of?(URI)
-      resolved = base_path.merge(path)
+    elsif base_path.kind_of?(Addressable::URI)
+      resolved = base_path.join(path)
       Premailer.canonicalize(resolved)
     elsif base_path.kind_of?(String) and base_path =~ /\A(?:(?:https?|ftp|file):)\/\//i
-      resolved = URI.parse(base_path)
-      resolved = resolved.merge(path)
+      resolved = Addressable::URI.parse(base_path)
+      resolved = resolved.join(path)
       Premailer.canonicalize(resolved)
     else
       File.expand_path(path, File.dirname(base_path))
@@ -500,7 +500,7 @@ def self.local_data?(data)
 
   # from http://www.ruby-forum.com/topic/140101
   def self.canonicalize(uri) # :nodoc:
-    u = uri.kind_of?(URI) ? uri : URI.parse(uri.to_s)
+    u = uri.kind_of?(Addressable::URI) ? uri : Addressable::URI.parse(uri.to_s)
     u.normalize!
     newpath = u.path
     while newpath.gsub!(%r{([^/]+)/\.\./?}) { |match|

test/helper.rb

@@ -9,7 +9,7 @@ class Premailer::TestCase < Minitest::Test
 
   def setup
     stub_request(:any, /premailer\.dev\/*/).to_return do |request|
-      file_path = BASE_PATH + URI.parse(request.uri).path
+      file_path = BASE_PATH + Addressable::URI.parse(request.uri).path
       if File.exists?(file_path)
         { :status => 200, :body => File.open(file_path) }
       else

test/test_links.rb

@@ -43,7 +43,7 @@ def test_appending_link_query_string
     premailer.processed_doc.search('a').each do |el|
       href = el.attributes['href'].to_s
       next if href.nil? or href.empty?
-      uri = URI.parse(href)
+      uri = Addressable::URI.parse(href)
       assert_match qs, uri.query, "missing query string for #{el.to_s}"
     end
 
@@ -111,21 +111,21 @@ def test_resolving_urls_from_string
   end
 
   def test_resolving_urls_from_uri
-    base_uri = URI.parse('http://example.com/')
+    base_uri = Addressable::URI.parse('http://example.com/')
     ['test.html', '/test.html', './test.html',
      'test/../test.html', 'test/../test/../test.html'].each do |q|
       assert_equal 'http://example.com/test.html', Premailer.resolve_link(q, base_uri), q
     end
 
-    base_uri = URI.parse('https://example.net:80/~basedir/')
+    base_uri = Addressable::URI.parse('https://example.net:80/~basedir/')
     assert_equal 'https://example.net:80/~basedir/test.html?var=1#anchor', Premailer.resolve_link('test/../test/../test.html?var=1#anchor', base_uri)
 
     # base URI with a query string
-    base_uri = URI.parse('http://example.com/dir/index.cfm?newsletterID=16')
+    base_uri = Addressable::URI.parse('http://example.com/dir/index.cfm?newsletterID=16')
     assert_equal 'http://example.com/dir/index.cfm?link=15', Premailer.resolve_link('?link=15', base_uri)
 
     # URI preceded by a space
-    base_uri = URI.parse('http://example.com/')
+    base_uri = Addressable::URI.parse('http://example.com/')
     assert_equal 'http://example.com/path', Premailer.resolve_link(' path', base_uri)
   end
 

test/test_misc.rb

@@ -254,7 +254,7 @@ def test_preserve_original_style_attribute
     style5_style = style5.attributes['style'].to_s
 
     assert_match /font-size: xx-large/, premailer.processed_doc.search('.style3').first.attributes['style'].to_s
-    assert_match /background: #000080/, style5_style
+    assert_match /background-color: #000080/, style5_style
     assert_match /text-align: center/, style5_style
     assert_match /#000080/, style5.attributes['bgcolor'].to_s
   end
@@ -299,23 +299,6 @@ def test_handling_shorthand_auto_properties
     assert_match /border-style: solid none solid solid;/, premailer.processed_doc.search('p').first.attributes['style'].to_s
   end
 
-  def test_sorting_style_attributes
-    html = <<END_HTML
-    <html>
-    <style type="text/css">
-      #page { right: 10px; left: 5px }
-    </style>
-    <body>
-      <div id='page'>test</div>
-    </body>
-    </html>
-END_HTML
-
-    premailer = Premailer.new(html, :with_html_string => true)
-    premailer.to_inline_css
-    assert_equal "left: 5px; right: 10px;", premailer.processed_doc.search('#page').first.attributes['style'].to_s
-  end
-
   def test_removing_scripts
     html = <<END_HTML
     <html>

test/test_premailer.rb

@@ -110,7 +110,7 @@ def test_css_to_attributes
       html = '<td style="background-color: #FFF;"></td>'
       premailer = Premailer.new(html, {:with_html_string => true, :adapter => adapter, :css_to_attributes => true})
       premailer.to_inline_css
-      assert_equal ';', premailer.processed_doc.search('td').first.attributes['style'].to_s
+      assert_equal '', premailer.processed_doc.search('td').first.attributes['style'].to_s
       assert_equal '#FFF', premailer.processed_doc.search('td').first.attributes['bgcolor'].to_s
     end
   end
@@ -120,7 +120,7 @@ def test_avoid_changing_css_to_attributes
       html = '<td style="background-color: #FFF;"></td>'
       premailer = Premailer.new(html, {:with_html_string => true, :adapter => adapter, :css_to_attributes => false})
       premailer.to_inline_css
-      assert_match /background: #FFF/, premailer.processed_doc.at_css('td').attributes['style'].to_s
+      assert_match /background-color: #FFF/, premailer.processed_doc.at_css('td').attributes['style'].to_s
     end
   end