Skip to content

Color not parsed correctly if rgb is not all lowercase #109

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

Closed
mganss opened this issue Mar 21, 2022 · 4 comments
Closed

Color not parsed correctly if rgb is not all lowercase #109

mganss opened this issue Mar 21, 2022 · 4 comments
Labels
Milestone

Comments

@mganss
Copy link
Contributor

mganss commented Mar 21, 2022

var html = @"<p style='color: RGB(0,17,0)'>Text</p>";
var parser = new HtmlParser(new HtmlParserOptions(), BrowsingContext.New(Configuration.Default.WithCss(new CssParserOptions())));
var dom = parser.ParseDocument(html);
var p = dom.QuerySelector("p");
var s = p.GetStyle();
var color = s.GetColor(); // -> ""

Perhaps this occurs because the dictionary here does not use a comparer that ignores case:

private static readonly Dictionary<String, Func<StringSource, Color?>> ColorFunctions = new Dictionary<String, Func<StringSource, Color?>>
{
{ FunctionNames.Rgb, ParseRgba },
{ FunctionNames.Rgba, ParseRgba },
{ FunctionNames.Hsl, ParseHsla },
{ FunctionNames.Hsla, ParseHsla },
{ FunctionNames.Gray, ParseGray },
{ FunctionNames.Hwb, ParseHwba },
{ FunctionNames.Hwba, ParseHwba },
};

This was originally reported as mganss/HtmlSanitizer#340.

@FlorianRappl
Copy link
Contributor

FlorianRappl commented Mar 21, 2022

It's a good point. Right now everything is case sensitive, but actually function names (and color names) should be case insensitive:

See https://www.w3.org/TR/css-color-4/#resolving-sRGB-values.

@mganss
Copy link
Contributor Author

mganss commented Mar 21, 2022

Isn't it the other way around, i.e. it should be case insensitive (rgb and RgB are treated the same, case is ignored) and currently it's case sensitive (only lowercase works)?

@FlorianRappl
Copy link
Contributor

Isn't it the other way around, i.e. it should be case insensitive (rgb and RgB are treated the same, case is ignored) and currently it's case sensitive (only lowercase works)?

Correct(ed) - that's what I meant.

@FlorianRappl FlorianRappl added this to the v0.17 milestone May 31, 2022
@FlorianRappl
Copy link
Contributor

Landed in devel.

FlorianRappl added a commit that referenced this issue Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants