peterbe / premailer

Turns CSS blocks into style attributes
https://premailer.io
BSD 3-Clause "New" or "Revised" License
1.06k stars 188 forks source link

Fix `merge_styles` for proper ordering of styles w.r.t. "constituent" styles #267

Open earshinov opened 2 years ago

earshinov commented 2 years ago

"Constituent" styles are, for example, margin-bottom for margin.

Rationale

To illustrate the problem, suppose you have this HTML:

<style>
    .c {
        display: inline-block;
        width: 50px;
        height: 50px;
        border: 5px solid red;
        border-bottom-width: 10px;
    }
</style>
<div class="c" style="border: 1px solid blue;"></div>
<!-- Effective style: border: 1px solid blue; -->

input

After "premailing" with the current version, this becomes

<html>
  <head></head>
  <body>
    <div class="c" style="display:inline-block; width:50px; height:50px; border:1px solid blue; border-bottom-width:10px" width="50" height="50"></div>
    <!-- Effective style: border: 1px solid blue; border-bottom-width: 10px; -->
  </body>
</html>

actual_output

what is obviously wrong. The desired output would be:

<html>
  <head></head>
  <body>
    <div class="c" style="display:inline-block; width:50px; height:50px; border-bottom-width:10px; border:1px solid blue" width="50" height="50"></div>
    <!-- Effective style: border: 1px solid blue; -->
  </body>
</html>

Note the ordering of border and border-bottom-width.

Suggested fix

The underlying reason for this error is that when you assign to an existing key in OrderedDict, it replaces the value in-place instead of appending it to the end. That is,

>>> import collections
>>> d = collections.OrderedDict((("a", 1), ("b", 2)))
>>> d["a"] = 3
>>> d
OrderedDict([('a', 3), ('b', 2)])

This way, the inline border style in the example takes place of the border style inherited from the .c class, and gets lesser priority than border-bottom-width.

The simplest fix to achieve the desired behavior would be to del the key from the OrderedDict first.

PR contents

PR contains a fix and a unit test failing without the fix.