ActiveCampaign / style-merge

Simple CSS inlining, for email.
MIT License
70 stars 10 forks source link

Bootstrap Css throws Exception #2

Open JPVenson opened 6 years ago

JPVenson commented 6 years ago

Hey,

When trying to Inline a Bootstrap 3.3.7 css file pasted into the Style tag, the Inliner throws an exception

System.ArgumentException: Unexpected character found at position 8: ".. button::>>-<<moz-focus-inner::input::-moz-"
   at CsQuery.StringScanner.Implementation.StringScannerEngine.ThrowException(String message, Int32 errPos)
   at CsQuery.StringScanner.Implementation.StringScannerEngine.ThrowUnexpectedCharacterException()
   at CsQuery.StringScanner.Implementation.StringScannerEngine.Expect(Func`3 validate)
   at CsQuery.StringScanner.Implementation.StringScannerEngine.Get(Func`3 validate)
   at CsQuery.Engine.SelectorParser.Parse(String selector)
   at CsQuery.Engine.Selector..ctor(String selector)
   at CsQuery.CQ.Select(String selector)
   at StyleMerge.Inliner.ApplyRulesToElements(CQ document, IEnumerable`1 rules, HashSet`1 noApply) in c:\projects\style-merge\StyleMerge\Inliner.cs:line 170
   at StyleMerge.Inliner.ProcessHtml(String sourceHtml) in c:\projects\style-merge\StyleMerge\Inliner.cs:line 145
   at JPB.MyWorksheet.Webpage.Services.Mail.MailService.PreProcessMail(Mail mail) in F:\Source\Repos\MyWorksheet\JPB.MyWorksheet\JPB.MyWorksheet.Webpage\Services\Mail\MailService.cs:line 48
   at JPB.MyWorksheet.Webpage.Services.Mail.MailService.<SendMail>d__18.MoveNext() in F:\Source\Repos\MyWorksheet\JPB.MyWorksheet\JPB.MyWorksheet.Webpage\Services\Mail\MailService.cs:line 63
EddieGreen commented 4 years ago

In Inliner - ApplyRulesToElements(), there is a specific error trap for pseudo classes relevant to browsers. In frameworks like foundation and bootstrap there are a ton of these, possibly unanticipated by the StyleMerge dev, for example

::-webkit-file-upload-button
button::-moz-focus-inner::type::-moz-focus-inner::type::-moz-focus-inner::type::-moz-focus-inner
.form select::-ms-expand

For some reason these pseudo-elements are being added to rules, and CsQuery generates an ArgumentException when getting document[rule.Selector].Elements in line 175.

The current catch block does not trap this error, in fact I'm not sure how useful the block is, seeing as it would be nicer to just move on to the next rule, maybe log the rules that won't play nicely.

Even better would be not to have these rules added in the first place. Now if I could only figure out how to do that...

You could replace the current catch block with:

catch(Exception ex)
{
    if(ex.GetType() == typeof(NotImplementedException))
    {
        if(ex.Message == "Pseudoclasses that require a browser aren't implemented.")
        {
            throw new InliningException($"Pseudo classes that require a browser, such as '{rule.Selector}', are not supported for inlining.");
        }
    }
}

Personally, I've opted to catch all errors and continue execution.