Unable to parse background-image

[niemyjski]

Bug Report

Prerequisites

• Can you reproduce the problem in a MWE?
• Are you running the latest version of AngleSharp?
• Did you check the FAQs to see if that helps you?
• Are you reporting to the correct repository? (there are multiple AngleSharp libraries, e.g., AngleSharp.Css for CSS support)
• Did you perform a search in the issues?

For more information, see the CONTRIBUTING guide.

Description

[Description of the bug]

I noticed that after painstakenly trying to upgrade from 0.9 to the latest release that some of my styles were completely removed. I stepped through the parser and found what I believe is a bug.

Steps to Reproduce

Take the following CSS

using AngleSharp;
using AngleSharp.Css.Parser;

var css = @"
nav {
    background-color: #FFFFFF;
    background-image: -webkit-gradient(linear,left 50%,left bottom,from(#FFFFFF),to(#eeeeee));
    background-image: -moz-linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);
    background-image: -ms-linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);
    background-image: -o-linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);
    background-image: linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);
    background-image: -webkit-linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);
    filter: progid:DXImageTransform.Microsoft.gradient(startColorStr='#FFFFFF',EndColorStr='#eeeeee');
}
";

var options = new CssParserOptions
{
    IsIncludingUnknownDeclarations = true,
    IsIncludingUnknownRules = true,
    IsToleratingInvalidSelectors = true
};
var context = BrowsingContext.New(Configuration.Default.WithCss(options));
var parser = new CssParser(options, context);
var parsed = parser.ParseStyleSheet(css);

Expected behavior: [What you expected to happen]
The above css rules are in the parsed result.

Actual behavior: [What actually happened]

nav { background-color: rgba(255, 255, 255, 1); filter: progid:DXImageTransform.Microsoft.gradient(startColorStr='#FFFFFF',EndColorStr='#eeeeee') }

Environment details: [OS, .NET Runtime, ...]
.NET 5, macOS (Latest)

Possible Solution

I think there is a logic bug in CSSParser.FillDeclarations (

AngleSharp.Css/src/AngleSharp.Css/Parser/CssBuilder.cs

Line 453 in 5d034bf

while (token.IsNot(CssTokenType.EndOfFile, CssTokenType.CurlyBracketClose))

)

If you look at it it calls CollectTrivia (ignores whitespace etc..) so the first style shows up, then calls CreateDeclarationWith(

AngleSharp.Css/src/AngleSharp.Css/Parser/CssBuilder.cs

Line 455 in 5d034bf

CreateDeclarationWith(style, ref token);

) which is at a valid token. Then it incorrectly calls CollectTrivia again. This happens in a loop of FillDeclarations to perfectly skip over valid rules.

[niemyjski]

Also, is there a case where I don't have to provide a context? And/or it feels pretty weird to have to specify the options twice when it's on the context.

[FlorianRappl]

I think your assumption is false. CollectTrivia is required to parse away potential comments and whitespaces. In case of non-trivia tokens it just returns.

If this would be the problem then most of the unit tests would be broken.

I think the problem is much more likely in the linear-gradient somewhat. If you replace that with, e.g., url('foo.png') it works better in the sense that the declaration is included.

In any case I think there is an issue here. Thanks for the report.

[FlorianRappl]

So yeah. The linear-gradient syntax is wrong. It need to be to top and not just top.

But since browsers still support this legacy syntax I've added it to AngleSharp.Css, too!

[niemyjski]

@FlorianRappl Thanks for the update, I upgraded to 0.15 and I ran the test above (even changed it to to top) and it doesn't even emit the linear-gradient. The parsed content is:

nav { 
  background-color: rgba(255, 255, 255, 1); 
  background-image: ; 
  filter: progid:DXImageTransform.Microsoft.gradient(startColorStr='#FFFFFF',EndColorStr='#eeeeee') 
}

I'd expect the content to round trip.

[FlorianRappl]

I'd expect the content to round trip.

HTML and CSS do not round trip. The serializations are not to preserve the original code, but rather to transport its interpretation.

Regarding the issue: There is even a passing test for this (https://github.com/AngleSharp/AngleSharp.Css/blob/devel/src/AngleSharp.Css.Tests/Declarations/CssBackgroundProperty.cs#L701) so not sure what the problem here is. Feel free to dig into that. Contributions welcome!

[niemyjski]

@FlorianRappl: I took the following modified test and debugged it some more. I noticed that the test CreateDeclarationWith creates the proper value for each declaration. The CreateValue call returns a cached property and then sets value. This seems to be the root issue because properties.SetProperty gets the existing valid declaration, then runs the declaration value through a converter (which returns null for vendor prefixes (e.g., webkit-)), finally it overwrites the value.

        [Test]
        public void CssBackgroundImageNotParsed_Issue66_ordering_matters_fails()
        {
            var source = @"
background-image: linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);
background-image: -webkit-linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);";

            var property = ParseDeclaration(source);
            Assert.IsTrue(property.HasValue);

            var expected = "linear-gradient(0deg, rgba(255, 255, 255, 1), rgba(255, 255, 255, 1), rgba(248, 248, 248, 1), rgba(238, 238, 238, 1))";
            Assert.AreEqual(expected, property.Value);
        }
        
        [Test]
        public void CssBackgroundImageNotParsed_Issue66_ordering_matters_works()
        {
            var source = @"
background-image: -webkit-linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);
background-image: linear-gradient(top,#FFFFFF,#FFFFFF,#f8f8f8,#eeeeee);";

            var property = ParseDeclaration(source);
            Assert.IsTrue(property.HasValue);

            var expected = "linear-gradient(0deg, rgba(255, 255, 255, 1), rgba(255, 255, 255, 1), rgba(248, 248, 248, 1), rgba(238, 238, 238, 1))";
            Assert.AreEqual(expected, property.Value);
        }

Seems like a fix may be if the value was not empty and the new value is empty don't set it? Previously to the latest version it returned all the declarations. Also it's kind of a what the heck is going on when you set a valid value and then it's empty. It took stepping into it to see it was running converters. Would be pretty nice to move the converters out of the setter.

[niemyjski]

@FlorianRappl Can you please reopen this issue.

[FlorianRappl]

I think the issue you posted is fixed. If you have a new issue / something else that is not working then please open another issue with a specific description.

Best case is that you also supply a PR that we can properly merge it in.

Thanks!

[niemyjski]

This wasn't fixed but I tried to dig into it without knowing the code base that well, I also added two tests above that could be copy and pasted.

[niemyjski]

^^ would it be possible to get the two tests I added to this project?