Additional handling in CssFormatter for transparent color cases (no h…

…ex representation)
ActiveCampaign · Jun 29, 2022 · 5e29212 · 5e29212
1 parent 01e22fe
commit 5e29212
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 22 deletions.
diff --git a/StyleMerge.Tests/InlinerTests.cs b/StyleMerge.Tests/InlinerTests.cs
@@ -34,6 +34,7 @@ public InlinerTests()
         [InlineData("ShouldApplyStylesForRulesWithMultipleSelectors")]
         [InlineData("ShouldConvertColorRGBAToHex")]
         [InlineData("ShouldDropTransparentBackgroundColor")]
+        [InlineData("ShouldDropInlineTransparentBackgroundColor")]
         [InlineData("ShouldEliminateStyleBlocksWhereAllRulesAreInlined")]
         [InlineData("ShouldEliminateScriptBlocks")]
         [InlineData("ShouldKeepImportStylesInStyleBlocks")]

diff --git a/StyleMerge.Tests/StyleMerge.Tests.csproj b/StyleMerge.Tests/StyleMerge.Tests.csproj
@@ -26,6 +26,8 @@
     <None Remove="TestCases\ShouldApplyStylesAccordingToSpecificityValues\Input.html" />
     <None Remove="TestCases\ShouldApplyStylesAccordingToSpecificityValues\Output.html" />
     <None Remove="TestCases\ShouldApplyStylesForRulesWithMultipleSelectors\Input.html" />
+    <None Remove="TestCases\ShouldDropInlineTransparentBackgroundColor\Input.html" />
+    <None Remove="TestCases\ShouldDropInlineTransparentBackgroundColor\Output.html" />
     <None Remove="TestCases\ShouldDropTransparentBackgroundColor\Input.html" />
     <None Remove="TestCases\ShouldDropTransparentBackgroundColor\Output.html" />
     <None Remove="TestCases\ShouldHandleBoxSizing\Input.html" />

diff --git a/StyleMerge.Tests/TestCases/ShouldConvertColorRGBAToHex/Input.html b/StyleMerge.Tests/TestCases/ShouldConvertColorRGBAToHex/Input.html
@@ -2,7 +2,7 @@
 <head>
     <style>
         div {
-            background-color: rgba(255, 0, 0, 1);
+            background-color: rgba(255, 0, 0, 0.75);
         }
     </style>
 </head>

diff --git a/StyleMerge.Tests/TestCases/ShouldDropInlineTransparentBackgroundColor/Input.html b/StyleMerge.Tests/TestCases/ShouldDropInlineTransparentBackgroundColor/Input.html
@@ -0,0 +1,16 @@
+<html>
+<head>
+    <style>
+        table {
+            font-size: 14px;
+        }
+    </style>
+</head>
+<body>
+    <table style="background-color: transparent">
+        <tbody>
+            <tr><td>Table content</td></tr>
+        </tbody>
+    </table>
+</body>
+</html>
diff --git a/StyleMerge.Tests/TestCases/ShouldDropInlineTransparentBackgroundColor/Output.html b/StyleMerge.Tests/TestCases/ShouldDropInlineTransparentBackgroundColor/Output.html
@@ -0,0 +1,11 @@
+<html>
+<head>
+</head>
+<body>
+    <table style="font-size: 14px">
+        <tbody>
+            <tr><td>Table content</td></tr>
+        </tbody>
+    </table>
+</body>
+</html>
diff --git a/StyleMerge/CssFormatter.cs b/StyleMerge/CssFormatter.cs
@@ -1,15 +1,18 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Text.RegularExpressions;
 using AngleSharp;
+using AngleSharp.Css.Dom;
+using AngleSharp.Css.Values;
 using AngleSharp.Text;
 
 namespace StyleMerge
 {
     public class CssFormatter : IStyleFormatter
     {
-        private const string RgbaPattern = @"rgba[(](\d{1,3})\s?,\s?(\d{1,3})\s?,\s?(\d{1,3})\s?,\s?(\d{1,3})[)]";
+        private const string RgbaPattern = @"rgba[(](\d{1,3})\s?,\s?(\d{1,3})\s?,\s?(\d{1,3}),\s?.*[)]";
         private static readonly MatchEvaluator RgbaEvaluator = new MatchEvaluator(ReplaceRgba);
 
         public string Declaration(string name, string value, bool important)
@@ -30,7 +33,7 @@ public string BlockDeclarations(IEnumerable<IStyleFormattable> declarations)
 
             using (var writer = new StringWriter(sb))
             {
-                foreach (var declaration in declarations)
+                foreach (var declaration in Filter(declarations))
                 {
                     writer.Write(Symbols.Space);
                     declaration.ToCss(writer, this);
@@ -55,7 +58,7 @@ public string BlockRules(IEnumerable<IStyleFormattable> rules)
 
             using (var writer = new StringWriter(sb))
             {
-                foreach (var rule in rules)
+                foreach (var rule in Filter(rules))
                 {
                     writer.Write(Symbols.Space);
                     rule.ToCss(writer, this);
@@ -89,7 +92,7 @@ public string Sheet(IEnumerable<IStyleFormattable> rules)
 
             using (var writer = new StringWriter(sb))
             {
-                foreach (var rule in rules)
+                foreach (var rule in Filter(rules))
                 {
                     rule.ToCss(writer, this);
                     writer.Write(sep);
@@ -112,5 +115,18 @@ private static string ReplaceRgba(Match match)
 
             return $"#{r:X2}{g:X2}{b:X2}";
         }
+
+        private IEnumerable<IStyleFormattable> Filter(IEnumerable<IStyleFormattable> rules)
+            => rules.Where(x => !IsTransparentColor(x));
+
+        private static bool IsTransparentColor(IStyleFormattable rule)
+        {
+            // If alpha channel is completely transparent (ie. background: transparent), we don't render
+            // this rule since there is no compatible 6-digit hex representation ... including this rule
+            // *should* not be necessary anyway, it is probably redundant in most cases.
+            return rule is ICssProperty prop
+                && prop.RawValue is Color color
+                && color.Alpha == 0;
+        }
     }
 }
diff --git a/StyleMerge/Inliner.cs b/StyleMerge/Inliner.cs
@@ -5,7 +5,6 @@
 using AngleSharp;
 using AngleSharp.Css.Dom;
 using AngleSharp.Css.Parser;
-using AngleSharp.Css.Values;
 using AngleSharp.Dom;
 using AngleSharp.Html.Dom;
 using AngleSharp.Html.Parser;
@@ -176,25 +175,25 @@ private static void ApplyRulesToElements(IHtmlDocument document, IEnumerable<Rul
 
             foreach (var rule in sortedRules)
             {
-                var applyProps = rule.Properties.Where(x => ShouldApply(x));
-                if (!applyProps.Any())
-                    continue;
-
                 try
                 {
                     // All elements except for those determined to not get styles.
                     foreach (var node in document.QuerySelectorAll(rule.Selector).Where(k => !noApply.Contains(k)))
                     {
                         var styles = CssParser.ParseDeclaration(node.GetAttribute("style") ?? string.Empty);
 
-                        foreach (var prop in applyProps)
+                        foreach (var prop in rule.Properties)
                         {
                             // TODO: We should probably lookup the existing property and only set it if it doesn't
                             // already exist. As things stand currently we are overwriting existing inline styles.
                             styles.SetProperty(prop.Name, prop.Value, prop.IsImportant ? "important" : null);
                         }
 
-                        node.SetAttribute("style", styles.ToCss(CssFormatter));
+                        var styleAttribute = styles.ToCss(CssFormatter);
+                        if (!string.IsNullOrWhiteSpace(styleAttribute))
+                        {
+                            node.SetAttribute("style", styleAttribute);
+                        }
                     }
                 }
                 catch (Exception ex)
@@ -203,15 +202,5 @@ private static void ApplyRulesToElements(IHtmlDocument document, IEnumerable<Rul
                 }
             }
         }
-
-        private static bool ShouldApply(ICssProperty x)
-        {
-            return !IsTransparentColor(x);
-        }
-
-        private static bool IsTransparentColor(ICssProperty prop)
-        {
-            return prop.RawValue is Color color && color.Alpha == 0;
-        }
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -2,7 +2,7 @@ @@
     <head>
         <style>
             div {
-                background-color: rgba(255, 0, 0, 1);
+                background-color: rgba(255, 0, 0, 0.75);
             }
         </style>
     </head>
@@ Expand Down @@