Repository: Automattic/juice
Pull request #385 (2021-05-14 04:02:41)

Fix !important added to table attributes
This fixes https://github.com/Automattic/juice/issues/356, which I encountered when cleaning up a marketing email today. I moved the regex to remove `!important` into a dedicated function, then referenced that function in a couple of places where the attributes are added.

This is my first commit to this repo, but I've tried to match the existing style and even added a test! Let me know if I'm missing anything. :)



Patch:
diff --git a/lib/inline.js b/lib/inline.js
index 866ee74..5a3c4cc 100644
--- a/lib/inline.js
+++ b/lib/inline.js
@@ -203,7 +203,7 @@ function inlineDocument($, css, options) {
             }
 
             var important = value.match(/!important$/) !== null;
-            if (important && !options.preserveImportant) value = value.replace(/\s*!important$/, '');
+            if (important && !options.preserveImportant) value = removeImportant(value);
             // adds line number and column number for the properties as "additionalPriority" to the
             // properties because in CSS the position directly affect the priority.
             var additionalPriority = [style[i].position.start.line, style[i].position.start.col];
@@ -289,13 +289,17 @@ function inlineDocument($, css, options) {
     if (juiceClient[dimension + 'Elements'].indexOf(elName) > -1) {
       for (var i in el.styleProps) {
         if (el.styleProps[i].prop === dimension) {
-          if (el.styleProps[i].value.match(/px/)) {
-            var pxSize = el.styleProps[i].value.replace('px', '');
+          var value = el.styleProps[i].value;
+          if (options.preserveImportant) {
+            value = removeImportant(value);
+          }
+          if (value.match(/px/)) {
+            var pxSize = value.replace('px', '');
             $(el).attr(dimension, pxSize);
             return;
           }
-          if (juiceClient.tableElements.indexOf(elName) > -1 && el.styleProps[i].value.match(/\%/)) {
-            $(el).attr(dimension, el.styleProps[i].value);
+          if (juiceClient.tableElements.indexOf(elName) > -1 && value.match(/\%/)) {
+            $(el).attr(dimension, value);
             return;
           }
         }
@@ -319,6 +323,9 @@ function inlineDocument($, css, options) {
         if (styleProps.indexOf(el.styleProps[i].prop) > -1) {
           var prop = juiceClient.styleToAttribute[el.styleProps[i].prop];
           var value = el.styleProps[i].value;
+          if (options.preserveImportant) {
+            value = removeImportant(value);
+          }
           if (prop === 'background') {
             value = extractBackgroundUrl(value);
           }
@@ -332,6 +339,10 @@ function inlineDocument($, css, options) {
   }
 }
 
+function removeImportant(value) {
+  return value.replace(/\s*!important$/, '')
+}
+
 function findVariableValue(el, variable) {
   while (el) {
     if (variable in el.styleProps) {


Test patch:
diff --git a/test/cases/juice-content/important.html b/test/cases/juice-content/important.html
index 0bd2bdf..2c05f93 100644
--- a/test/cases/juice-content/important.html
+++ b/test/cases/juice-content/important.html
@@ -7,11 +7,16 @@
         span {
             color: blue;
         }
+        table {
+          background-color: black !important;
+          width: 100% !important;
+        }
     </style>
 </head>
 <body>
     <p>hi</p>
     <span>there</span>
     <p style="color: blue !important">again</p>
+    <table></table>
 </body>
 </html>
diff --git a/test/cases/juice-content/important.out b/test/cases/juice-content/important.out
index 214d21c..8412b7c 100644
--- a/test/cases/juice-content/important.out
+++ b/test/cases/juice-content/important.out
@@ -6,5 +6,6 @@
     <p style="color: red !important;">hi</p>
     <span style="color: blue;">there</span>
     <p style="color: blue !important;">again</p>
+    <table style="background-color: black !important; width: 100% !important;" width="100%" bgcolor="black"></table>
 </body>
 </html>