search

premailer / css_parser

Ruby CSS Parser
Other
279 stars 110 forks source link

Ensure order key exists in declaration array for all property short to long conversions #80

Closed EBashkoff closed 7 years ago

EBashkoff commented 7 years ago

If an :order key does not exist on a source or destination @declaration, then the comparison for order fails because we are comparing nil to an integer.

This pertains to issue https://github.com/premailer/css_parser/issues/81

EBashkoff commented 7 years ago

The error can be reproduced with html_body set to the attached HTML and the following commands:

premailer = Premailer.new(html_body, {
        preserve_styles: true,
        adapter: :nokogiri,
        with_html_string: true,
        input_encoding: "UTF-8"
      })
premailer.to_inline_css

The proposed code change seems to solve this problem.

htmlbody.html.zip

grosser commented 7 years ago

I might break this in the future :trollface: ... can you add a testcase that fails without this ?

EBashkoff commented 7 years ago

@grosser If you unzip the attachment using that HTML as html_body and run the #to_inline_css method on the premailer configured using the options I showed above, it raises the error (on premailer v1.8.7 and css_parser v1.4.5).

grosser commented 7 years ago

can you pack this into a test so it will be re-checked automatically instead of once manually ?

EBashkoff commented 7 years ago

@grosser My apologies. I was able to reproduce the problem reducing the original HTML I had attached to the following HTML:

<head>
  <style type="text/css">
  table td {
    border-collapse: collapse;
  }
</style>
</head>
<body>
  <table style="border-collapse: collapse;">
    <table style="border-collapse: collapse;">
      <tr>
        <td style="border-bottom: 5px solid #F77704;border-color: #000000;"></td>
      </tr>
    </table>
  </table>
</body>

but when I went to write the tests to generate the error, I realized the error was not reproduced with your current version 1.4.7 (and my app was on 1.4.5). It looks like my PR is very similar to this commit that was included in v1.4.6: https://github.com/premailer/css_parser/commit/5d0ea458fe243d14a9b0090f42cb53f0a55145ff

Thanks for your help! I will close this PR and the issue I raised.