A warning when a css resource could not be downloaded & apply styles only to elements within (and itself) the body #395
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
February 2, 2024 15:15
…ied to the html element, head element and elements in the head like script, link etc. To avoid this behaviour only the elements from the body (and itself) are selected to be matched.
…eption when a resource could not be downloaded.
horch004
changed the title
Comment on lines
+254
to
+262
| try | ||
| { | ||
| return cssSource.GetCss(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _warnings.Add(ex.Message); | ||
| return string.Empty; | ||
| } |
There was a problem hiding this comment.
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?
| @@ -405,7 +417,7 @@ private void RemoveStyleElements(IEnumerable<IElement> cssSourceNodes, bool pres | |||
| Selector = selectorParser.ParseSelector(x.Value.Name) | |||
| }).Where(x => x.Selector != null).ToList(); | |||
|
|
|||
| foreach (var el in _document.DescendentsAndSelf<IElement>()) | |||
| foreach (var el in _document.Body.DescendentsAndSelf<IElement>()) | |||
There was a problem hiding this comment.
I'm unsure if we create problems by not allowing to target the html tag? I cannot rule out the use of obscure hacks on the html element for some email clients is necessary?
Would need some more research to be honest. You could also avoid the style tags on those elements, with body *?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Let GetCssBlocks add a warning to the list instead of throwing an exception when a resource could not be downloaded.
With an All (*) selector in the stylesheet, the css rule is also applied to the html element, head element and elements in the head like script, link etc. To avoid this behaviour only the elements from the body (and itself) are selected to be matched.
Maybe there is another way not to match the all selector to the elements above the body, but this was a quick fix for us.