Fix #410: Preserve !important tag in inlined styles #424

devin-ai-integration · 2025-05-21T04:55:19Z

Preserve !important tag in inlined styles

This PR fixes issue #410 where the !important tag is being removed from inlined styles when using premailer.

Problem

When a class matches a style, PreMailer.Net incorrectly removes !important from styles that are unrelated to the class being inlined.

Example:
Input:

<style> 
.test {
 color:red;
 }
 </style>
<body>
<p class="test" style="font-weight: bold !important;">test</p>
</body>

Output (incorrect):

<body>
<p class="test" style="color:red; font-weight: bold;">test</p>
</body>

Fix

The fix modifies the following:

CssElementStyleResolver.GetAllStyles() to pass emitImportant: true when calling styleClass.ToString(), preserving the !important flag in stylesheet styles.
StyleClass.Merge method to handle all cases correctly, ensuring that important styles are preserved during merging.
StyleClassApplier.ApplyStyles method to preserve the existing inline style with !important when adding new styles.

Testing

Added a test case to verify that !important tags are preserved in the inlined output.

Link to Devin run: https://app.devin.ai/sessions/cdcd17119f824643ace0e832bba5701b
Requested by: m@martinnormark.com

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

devin-ai-integration · 2025-05-21T04:55:23Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment and CI monitoring

PreMailer.Net/PreMailer.Net/StyleClass.cs

PreMailer.Net/PreMailer.Net/PreMailer.cs

PreMailer.Net/PreMailer.Net/StyleClassApplier.cs

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

devin-ai-integration · 2025-05-21T12:30:04Z

The newline at the end of file change was not intentional and is not needed for the functionality. It was an automatic formatting change that happened when editing the file. This doesn't affect the behavior of the code.

PreMailer.Net/PreMailer.Net/StyleClassApplier.cs

devin-ai-integration · 2025-05-21T12:32:50Z

The additional code is needed because the existing style merging logic doesn't properly handle !important flags in inline styles. The issue occurs when a style with an !important flag is present in the original inline style (like font-weight: bold !important), but gets overwritten during the style application process.

The existing code applies all styles from the stylesheet and then checks if the resulting style attribute contains !important. However, by that point, the original !important flags might have been lost.

This additional logic specifically:

Captures the original inline style before any modifications
Checks if it contains any !important flags
After applying all styles, it checks if the !important flags were lost
If they were lost, it parses both the original and current styles, and ensures that any attributes with !important from the original style are preserved

This is necessary to fix issue #410 where !important flags in inline styles were being removed during the inlining process.

martinnormark · 2025-05-21T12:41:05Z

The existing code applies all styles from the stylesheet and then checks if the resulting style attribute contains !important. However, by that point, the original !important flags might have been lost.

Can you make a unit test to prove this, then try to remove this new logic after writing the test. If the test still pass, the new logic can be discarded.

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

devin-ai-integration · 2025-05-21T12:45:23Z

I've created a test case that reproduces the issue described in #410 and tested whether the additional logic in StyleClassApplier.cs is necessary.

Test Results:

With the additional logic: All tests pass (153/153)
Without the additional logic: Test fails with the following error:

Assert.Equal() Failure: Strings differ
Expected: ..."v style="color: #000;font-weight: bold !i"...
Actual:   "<div style="color: #000"></div>"

This proves that the additional logic is necessary to preserve the !important flag in inline styles. When the logic is removed, the !important flag is lost during the style application process.

I've added a specific test case in PreMailerTests.cs that reproduces the exact scenario from issue #410 to ensure this behavior is properly tested.

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

PreMailer.Net/PreMailer.Net.Tests/Issue410Tests.cs

…ith descriptive name Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

devin-ai-integration · 2025-05-21T12:53:43Z

I've addressed your feedback by:

Removing the Issue410Tests.cs file with vague naming
Moving the test to PreMailerTests.cs with a more descriptive name: MoveCssInline_ShouldPreserveImportantFlagInInlineStyles_WhenApplyingClassStyles
Verifying that all tests still pass (152/152)

The test now clearly conveys its purpose without referencing the issue number directly.

…StyleResolver Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

devin-ai-integration · 2025-05-21T13:31:36Z

I've moved the logic for preserving !important flags from StyleClassApplier.ApplyStyles() to CssElementStyleResolver.GetAllStyles() as suggested. This improves the architecture by placing the logic in a more appropriate location, since GetAllStyles() is responsible for collecting all styles that should be applied to an element.

The changes:

Modified CssElementStyleResolver.GetAllStyles() to extract the inline style from the DOM element
Added logic to preserve !important flags from the original inline style
Simplified StyleClassApplier.ApplyStyles() to remove the duplicate logic

All tests are still passing (152/152), confirming that the functionality is preserved.

PreMailer.Net/PreMailer.Net/CssElementStyleResolver.cs

… add comprehensive tests Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

devin-ai-integration · 2025-05-21T15:48:11Z

I've simplified the logic in CssElementStyleResolver.GetAllStyles() as suggested and added comprehensive tests to cover the entire API.

Changes made:

Added two unit tests to CssElementStyleResolverTests.cs:
- GetAllStyles_PreservesImportantFlagsInInlineStyles - Tests that important flags in inline styles are preserved
- GetAllStyles_PreservesImportantFlagsWhenMergingStyles - Tests that important flags are preserved when merging styles with the same property
Simplified the logic in CssElementStyleResolver.GetAllStyles() by:
- Merging the original style with the styleClass parameter before adding it to the attributeCssList
- Eliminating the need for post-processing
- Properly handling important flags in both the original style and the styleClass

The simplified approach:

Checks if there's an original inline style
If so, creates a merged StyleClass that combines both the styleClass parameter and the original inline style
Ensures that important flags are preserved during the merge
Adds the merged style to the result

All tests are passing (154/154), confirming that the functionality is preserved while making the code more maintainable.

PreMailer.Net/PreMailer.Net/CssElementStyleResolver.cs

…lse if block Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

devin-ai-integration · 2025-05-21T16:14:23Z

I've further simplified the logic in CssElementStyleResolver.GetAllStyles() as suggested by avoiding the else if block.

Changes made:

Always create a mergedStyleClass regardless of whether there's an original inline style or not
Merge the styleClass into it (if it has attributes)
Merge the originalStyleAttr into it (if it exists)
Add the mergedStyleClass to the attributeCssList (if it has attributes)

This approach eliminates the duplication and makes the code more elegant while maintaining the same functionality. All tests are still passing (154/154).

devin-ai-integration bot and others added 2 commits May 21, 2025 04:54

Fix #410: Preserve !important tag in inlined styles

d69942c

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

Fix #410: Minor code formatting in PreMailer.cs

9074ea4

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

martinnormark requested changes May 21, 2025

View reviewed changes

PreMailer.Net/PreMailer.Net/StyleClass.cs Outdated Show resolved Hide resolved

PreMailer.Net/PreMailer.Net/PreMailer.cs Outdated Show resolved Hide resolved

PreMailer.Net/PreMailer.Net/StyleClassApplier.cs Show resolved Hide resolved

Address PR feedback: Fix code style issues

5f1db61

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

martinnormark reviewed May 21, 2025

View reviewed changes

PreMailer.Net/PreMailer.Net/StyleClassApplier.cs Outdated Show resolved Hide resolved

Add test case for issue #410 to prove the need for additional logic

1c60494

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

Add dedicated test case for issue #410

bfeec2b

Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

martinnormark reviewed May 21, 2025

View reviewed changes

PreMailer.Net/PreMailer.Net.Tests/Issue410Tests.cs Outdated Show resolved Hide resolved

Address PR feedback: Move test from Issue410Tests to PreMailerTests w…

b4f4e1c

…ith descriptive name Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

Address PR feedback: Move !important preservation logic to CssElement…

37ba464

…StyleResolver Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

martinnormark reviewed May 21, 2025

View reviewed changes

PreMailer.Net/PreMailer.Net/CssElementStyleResolver.cs Outdated Show resolved Hide resolved

Simplify !important preservation logic in CssElementStyleResolver and…

1045232

… add comprehensive tests Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

martinnormark reviewed May 21, 2025

View reviewed changes

PreMailer.Net/PreMailer.Net/CssElementStyleResolver.cs Outdated Show resolved Hide resolved

Further simplify CssElementStyleResolver.GetAllStyles() by avoiding e…

72e6c77

…lse if block Co-Authored-By: m@martinnormark.com <m@martinnormark.com>

martinnormark approved these changes May 21, 2025

View reviewed changes

martinnormark merged commit a34e7a7 into main May 21, 2025
2 checks passed

martinnormark deleted the devin/1747802554-preserve-important-flag branch May 21, 2025 16:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix #410: Preserve !important tag in inlined styles #424

Fix #410: Preserve !important tag in inlined styles #424

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

martinnormark commented May 21, 2025

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

Uh oh!

Fix #410: Preserve !important tag in inlined styles #424

Fix #410: Preserve !important tag in inlined styles #424

Uh oh!

Conversation

devin-ai-integration bot commented May 21, 2025

Preserve !important tag in inlined styles

Problem

Fix

Testing

Uh oh!

devin-ai-integration bot commented May 21, 2025

🤖 Devin AI Engineer

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

martinnormark commented May 21, 2025

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

devin-ai-integration bot commented May 21, 2025

Uh oh!

Uh oh!

Uh oh!