Skip to content
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

CSS variables flagged as Parse Error #644

Closed
morganwdavis opened this issue Apr 20, 2018 · 6 comments

Comments

@morganwdavis
Copy link

commented Apr 20, 2018

Valid CSS variables for user agent are flagged as CSS Parse Error.

@sideshowbarker

This comment has been minimized.

Copy link
Member

commented Apr 20, 2018

Please indicate what checker you’re testing with — https://validator.w3.org/nu/ or https://checker.html5.org/ or that vnu.jar command line checker? — and also please post a URL or document that I can test with.

I’ve testing myself at https://validator.w3.org/nu/ and https://checker.html5.org/with the following document that contains CSS variables:

https://validator.w3.org/nu/?showsource=yes&doc=data%3Atext%2Fhtml%3Bcharset%3Dutf-8%2C%253C%2521DOCTYPE%2520html%253E%250D%250A%253Chtml%2520lang%253D%2522%2522%253E%250D%250A%253Chead%253E%250D%250A%253Ctitle%253ETest%253C%252Ftitle%253E%250D%250A%253Cstyle%253E%250D%250Aelement%2520%257B%250D%250A%2520%2520--main-bg-color%253A%2520brown%253B%250D%250A%257D%250D%250Aelement%2520%257B%250D%250A%2520%2520background-color%253A%2520var%2528--main-bg-color%2529%253B%250D%250A%257D%250D%250A%253C%252Fstyle%253E%2520

https://checker.html5.org/?showsource=yes&doc=data%3Atext%2Fhtml%3Bcharset%3Dutf-8%2C%253C%2521DOCTYPE%2520html%253E%250D%250A%253Chtml%2520lang%253D%2522%2522%253E%250D%250A%253Chead%253E%250D%250A%253Ctitle%253ETest%253C%252Ftitle%253E%250D%250A%253Cstyle%253E%250D%250Aelement%2520%257B%250D%250A%2520%2520--main-bg-color%253A%2520brown%253B%250D%250A%257D%250D%250Aelement%2520%257B%250D%250A%2520%2520background-color%253A%2520var%2528--main-bg-color%2529%253B%250D%250A%257D%250D%250A%253C%252Fstyle%253E%2520

java -jar ./build/dist/vnu.jar 'data:text/html;charset=utf-8,%3C%21DOCTYPE%20html%3E%0D%0A%3Chtml%20lang%3D%22%22%3E%0D%0A%3Chead%3E%0D%0A%3Ctitle%3ETest%3C%2Ftitle%3E%0D%0A%3Cstyle%3E%0D%0Aelement%20%7B%0D%0A%20%20--main-bg-color%3A%20brown%3B%0D%0A%7D%0D%0Aelement%20%7B%0D%0A%20%20background-color%3A%20var%28--main-bg-color%29%3B%0D%0A%7D%0D%0A%3C%2Fstyle%3E%20'

…and in all cases, no errors are reported (as expected).

@morganwdavis

This comment has been minimized.

Copy link
Author

commented Apr 21, 2018

I tried it with both sites and I get the same results (more errors on checker.html5.org). Here's the site I tested it with: https://www.jeromes.com. It has minified, inlined CSS, with fairly complex calc()s for computing varying flex layouts with gutters.

Rather than trying to interpret that, here's a tiny test that fails the same way:

#test {
  --x: 5;
  --y: 3;
  --z: calc(100% / var(--x) - var(--y));
}

Regardless of the validator's report, Chrome is able to interpret this properly.

sideshowbarker added a commit to w3c/css-validator that referenced this issue May 6, 2018

sideshowbarker added a commit that referenced this issue May 6, 2018

@sideshowbarker

This comment has been minimized.

Copy link
Member

commented May 6, 2018

I’ve “fixed” the behavior for the case in the snippet in #644 (comment). See https://goo.gl/6XChFZ

However, I think there are other cases that can still fail, and I’m not super-confident the change I made doesn’t introduce any regressions or otherwise have unintended side effects.

The thing is, the current CSS variables “support” in the checker is a kind of crude hack that’s just meant as an interim workaround until we manage to get it implemented in the right way. I’m still a bit surprised I managed to get it working at all…

But anyway, if it works at least for the documents you’re checking, then I guess we could go ahead and close this issue. So lemme know.

And then If you later run into a case later that it doesn’t work for, then you could open up a new issue.

sideshowbarker added a commit to w3c/css-validator that referenced this issue May 7, 2018

@morganwdavis

This comment has been minimized.

Copy link
Author

commented May 9, 2018

Seems to work with my code, and reports no issues now. I also threw some error cases at it, such as lack of spacing in calc(), invalid variable name syntax, etc, and it caught them all properly. Thanks!

@sideshowbarker

This comment has been minimized.

Copy link
Member

commented May 9, 2018

OK — I’ll go ahead and close this.

Thanks much for having taken time to report the problem. I’m glad I was able to actually fix it for your cases with a relatively minimal amount of re-wrestling with the code.

Anyway, if/when you do run into any new cases that it chokes on, please open new issues.

sideshowbarker added a commit to w3c/css-validator that referenced this issue Jun 5, 2018

sideshowbarker added a commit to w3c/css-validator that referenced this issue Jun 7, 2018

sideshowbarker added a commit to w3c/css-validator that referenced this issue Jun 7, 2018

sideshowbarker added a commit to w3c/css-validator that referenced this issue Jun 7, 2018

sideshowbarker added a commit to w3c/css-validator that referenced this issue Jun 25, 2018

sideshowbarker added a commit to w3c/css-validator that referenced this issue Jul 30, 2018

sideshowbarker added a commit to w3c/css-validator that referenced this issue Aug 26, 2018

sideshowbarker added a commit to w3c/css-validator that referenced this issue Aug 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.