Skip to content

A warning when a css resource could not be downloaded & apply styles only to elements within (and itself) the body #395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion PreMailer.Net/PreMailer.Net/PreMailer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,19 @@
/// </summary>
private IEnumerable<string> GetCssBlocks(IEnumerable<ICssSource> cssSources)
{
return cssSources.Select(styleSource => styleSource.GetCss()).ToList();
return cssSources.Select(GetCssBlock).ToList();
}
private string GetCssBlock(ICssSource cssSource)
{
try
{
return cssSource.GetCss();
}
catch (Exception ex)
{
_warnings.Add(ex.Message);
return string.Empty;
}
Comment on lines +254 to +262
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm honestly a bit torn on this change. If your html needs a CSS source and it is not available, aren't we breaking things by ignoring that and silently moving ahead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused. When a stylesheet contains bogus or bad css the parser silently ignores it, but when the url is not found (e.g. someone deleted it) the whole process exits with an exception?
The purpose of adding a warning is that the caller can know something is wrong, but that the process of inlining is not stopped. There might be other stylesheets and/or style nodes involved that can be processed.

Now we have to proceed with emailing without any inline style when such an exception occurs because a client forgot to edit his email template after removing a stylesheet from its website.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(horch004 is my private github. whorchner from my work)

}

private void RemoveCssComments(IEnumerable<IElement> cssSourceNodes)
Expand Down Expand Up @@ -405,7 +417,7 @@
Selector = selectorParser.ParseSelector(x.Value.Name)
}).Where(x => x.Selector != null).ToList();

foreach (var el in _document.DescendentsAndSelf<IElement>())

Check warning on line 420 in PreMailer.Net/PreMailer.Net/PreMailer.cs

View workflow job for this annotation

GitHub Actions / build

'ParentNodeExtensions.DescendentsAndSelf<TNode>(INode)' is obsolete: 'Use DescendantsAndSelf'

Check warning on line 420 in PreMailer.Net/PreMailer.Net/PreMailer.cs

View workflow job for this annotation

GitHub Actions / build

'ParentNodeExtensions.DescendentsAndSelf<TNode>(INode)' is obsolete: 'Use DescendantsAndSelf'

Check warning on line 420 in PreMailer.Net/PreMailer.Net/PreMailer.cs

View workflow job for this annotation

GitHub Actions / build

'ParentNodeExtensions.DescendentsAndSelf<TNode>(INode)' is obsolete: 'Use DescendantsAndSelf'

Check warning on line 420 in PreMailer.Net/PreMailer.Net/PreMailer.cs

View workflow job for this annotation

GitHub Actions / build

'ParentNodeExtensions.DescendentsAndSelf<TNode>(INode)' is obsolete: 'Use DescendantsAndSelf'
{
foreach (var style in styles)
{
Expand Down Expand Up @@ -516,7 +528,7 @@

private void RemoveHtmlComments()
{
var comments = _document.Descendents<IComment>().ToList();

Check warning on line 531 in PreMailer.Net/PreMailer.Net/PreMailer.cs

View workflow job for this annotation

GitHub Actions / build

'ParentNodeExtensions.Descendents<TNode>(INode)' is obsolete: 'Use Descendants'

Check warning on line 531 in PreMailer.Net/PreMailer.Net/PreMailer.cs

View workflow job for this annotation

GitHub Actions / build

'ParentNodeExtensions.Descendents<TNode>(INode)' is obsolete: 'Use Descendants'

Check warning on line 531 in PreMailer.Net/PreMailer.Net/PreMailer.cs

View workflow job for this annotation

GitHub Actions / build

'ParentNodeExtensions.Descendents<TNode>(INode)' is obsolete: 'Use Descendants'

foreach (var comment in comments)
{
Expand Down
Loading