Repository: peterbe/premailer

Issue:
Style sorting breaking proper precedence
For the following styles:
```
.foo {
  padding-left: 6px;
  padding-right: 6px;
  padding-top: 8px;
  padding-bottom: 8px;
  padding: 4px;
}
```
The browser should see the padding rules as all having the same precedence. As a tie-breaker, whichever appears last in the list applies (in this case `padding: 4px`).

Premailer appears to be [sorting styles alphabetically in `csstext_to_pairs`](https://github.com/peterbe/premailer/blob/9cbd27afa4fa81bb84219d091e391ef4e7109fe0/premailer/merge_style.py#L25). This causes the `padding` rule to appear before the others and breaks the precedence. 

Is there any reason to be sorting styles in this way? Would you be open to a PR removing this sort?

```
> /virtualenv_run/lib/python3.7/site-packages/premailer/premailer.py(473)transform()
-> for item in items:
(Pdb) style
'font-family:"Open Sans", "Helvetica Neue", Helvetica, Arial, sans-serif;font-weight:normal;font-size:14px;line-height:20px;color:rgba(43, 39, 60, 1);text-align:left;margin-bottom:16px;padding-left:6px;padding-right:6px;padding-top:8px;padding-bottom:8px;padding:5px'
(Pdb) processed_style
[('color', 'rgba(43, 39, 60, 1)'), ('font-family', '"Open Sans", "Helvetica Neue", Helvetica, Arial, sans-serif'), ('font-size', '14px'), ('font-weight', 'normal'), ('line-height', '20px'), ('margin-bottom', '16px'), ('padding', '5px'), ('padding-bottom', '8px'), ('padding-left', '6px'), ('padding-right', '6px'), ('padding-top', '8px'), ('text-align', 'left')]
```

Discussion:
peterbe: @jack-guy if you wouldn't mind, please don't rebase or squash the commits. Otherwise I can't see what changed. 
jack-guy: Happy to not rebase @peterbe but also pro tip you can click the "force-pushed" to see just the changes :)
peterbe: > Happy to not rebase @peterbe but also pro tip you can click the "force-pushed" to see just the changes :)
> ![image](https://user-images.githubusercontent.com/957238/127917879-6ced1c24-b59e-4676-b096-facfce39dc23.png)

[mind-blown-gif] !

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index debbd61..1c6fa58 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -7,6 +7,25 @@ never too late to start, so let's start here and now.
 Unreleased
 ----------
 * New option ``session=None`` to provide the session used for making http requests.
+* Bug fix: inlined styles are no longer sorted alphabetically. This preserves the input
+  rule order so that premailer does not break style precedence where order is significant, e.g.
+
+  .. code-block:: css
+
+    div {
+      /* Padding on all sides is 10px. */
+      padding-left: 5px;
+      padding: 10px;
+    }
+
+    div {
+      /* Padding on the left side is 5px, on other sides is 10px. */
+      padding: 10px;
+      padding-left: 5px;
+    }
+
+  Prior to this fix premailer would swap the rules in the first example to look like the second.
+
 
 3.9.0
 -----
diff --git a/premailer/merge_style.py b/premailer/merge_style.py
index 37ea153..62d4a9d 100644
--- a/premailer/merge_style.py
+++ b/premailer/merge_style.py
@@ -1,6 +1,5 @@
 import cssutils
 import threading
-from operator import itemgetter
 from collections import OrderedDict
 
 from premailer.cache import function_cache
@@ -22,13 +21,10 @@ def csstext_to_pairs(csstext, validate=True):
     # The lock is required to avoid ``cssutils`` concurrency
     # issues documented in issue #65
     with csstext_to_pairs._lock:
-        return sorted(
-            [
-                (prop.name.strip(), format_value(prop))
-                for prop in cssutils.parseStyle(csstext, validate=validate)
-            ],
-            key=itemgetter(0),
-        )
+        return [
+            (prop.name.strip(), format_value(prop))
+            for prop in cssutils.parseStyle(csstext, validate=validate)
+        ]
 
 
 csstext_to_pairs._lock = threading.RLock()


Test patch:
diff --git a/premailer/tests/test_premailer.py b/premailer/tests/test_premailer.py
index b4e4180..c1e803e 100644
--- a/premailer/tests/test_premailer.py
+++ b/premailer/tests/test_premailer.py
@@ -710,8 +710,8 @@ class Tests(unittest.TestCase):
         <head>
         <title>Title</title>
         </head>
-        <body style="background:url(http://exam
-ple.com/bg.png); color:#123; font-family:Omerta">
+        <body style="color:#123; background:url(http://exam
+ple.com/bg.png); font-family:Omerta">
         <h1>Hi!</h1>
         </body>
         </html>""".replace(
@@ -933,7 +933,7 @@ ical-align:middle" bgcolor="red" valign="middle">Cell 2</td>
         </head>
         <body>
         <p style="text-align:center">Text</p>
-        <table style="height:300px; width:200px">
+        <table style="width:200px; height:300px">
           <tr>
             <td style="background-color:red" bgcolor="red">Cell 1</td>
             <td style="background-color:red" bgcolor="red">Cell 2</td>
@@ -1052,7 +1052,7 @@ b
 </head>
 <body>
 <h1 style="Margin:0">a</h1>
-<h2 style="Margin-bottom:0; Margin-left:0; Margin-right:0; Margin-top:0">
+<h2 style="Margin-top:0; Margin-bottom:0; Margin-left:0; Margin-right:0">
 b
 </h2>
 </body>
@@ -1260,6 +1260,39 @@ b
 
         compare_html(expect_html, result_html)
 
+    def test_css_ordering_preserved(self):
+        """For cases like these padding rules, it's important that the style that
+        should be applied comes last so that premailer follows the same rules that
+        browsers use to determine precedence."""
+
+        html = """<html>
+        <head>
+        <style type="text/css">
+        div {
+            padding-left: 6px;
+            padding-right: 6px;
+            padding: 4px;
+        }
+        </style>
+        </head>
+        <body>
+        <div>Some text</div>
+        </body>
+        </html>"""
+
+        expect_html = """<html>
+        <head>
+        </head>
+        <body>
+        <div style="padding-left:6px; padding-right:6px; padding:4px">Some text</div>
+        </body>
+        </html>"""
+
+        p = Premailer(html)
+        result_html = p.transform()
+
+        compare_html(expect_html, result_html)
+
     def test_doctype(self):
         html = (
             '<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" '