issues parsing invalid declarations inside phantomjs

[pocketjoso]

Hi there, thanks for a great tool, using it for penthouse since a few versions back.

Not sure where this fits, whether this is something that can/should be fixed or not, as issue only happens inside phantomjs. Any input on how to resolve it, even if fix is outside of this repo much appreciated. Basically I came across this invalid css which does *not* generate anyparsingError, but does throw off the parser so as to make it fail to detect any following rules (after the.breakingDeclaration), which causespenthouse` to lose part of critical css in this scenario.

Setup

I'm on version 2.0.0 of css (couldn't get more recent version to work in phantom, worth it's own issue..).

Call css.parse() on the following css* inside a phantomjs script (I've tried both 1.9.8 and 2.0.0 - same result):

.breakingDeclaration{filter:unquote("()")}html{}

To clarify, this call is made inside the top level body of a phantomjs script, not inside any page.evaluate js injected to headless browser. The actual line is here.

Expected

• two rules in stylesheet (.breakingDeclaration and html)

-or-

• parsingError for the .breakingDeclaration declaration

Actual

only the .breakingDeclaration rule (and any preceeding ones) is kept. html (and any following) rules are lost.

I guess it's the double nested set of () inside the declaration that throws of a regex somewhere? Note too that I spotted that even though the rules are lost in the AST rules array, calling stringify with the same array somehow adds the missing rules back again. Doesn't help me though.

*The actual declaration was filter:unquote("progid:DXImageTransform.Microsoft.gradient(enabled = false)") - some left behind directive, dunno what language.

[conradz]

This seems to have been fixed in the latest version of css. Please re-open if it still occurs in the latest version 2.2.1. If you're running an older version you can expect that there will be bugs that have been fixed.

[pocketjoso]

@conradz Did you actually test that it works inside phantomjs with css@2.2.1? As I mentioned I had issues using that version inside phantomjs last time I tried, but will try again now.

[pocketjoso]

So @conradz I tested this myself now in css@2.2.1 (I had to manually revert this commit, as it doesn't work in phantomjs) - and it doesn't work in this version either, in phantomjs.

with this test css:
.breakingDeclaration{filter:unquote("()")}html{color:red;}div{color:blue;}

outside phantomjs -- correct

css.parse outside of phantomjs produces an ast with rules.length 3:

.breakingDeclaration
property: filter
value: unquote("()")

html
property: color
value: red

div
property: color
value: blue

inside phantomjs -- incorrect

whereas inside phantomjseverything gets incorrectly collapsed into the broken declaration, ast rules.length 1:

.breakingDeclaration
property: filter
value: unquote("()")}html{color:red;}div{color:blue

---

Like I said from the beginning, I'm not sure if there's anything you can do about this form your libraries point of view. Just wanted to clarify that is doesn't get fixed by latest version.

Cheers

[pocketjoso]

I just don't have enough understanding of what css.parse does that could yield a different result when run inside phantomjs. I might just rewrite my usage so I do the parsing outside of phantomjs, seralise the ast, and then pass it in.

[conradz]

OK, thanks for the clarification, I hadn't understood that it was specific to phantomjs. Yeah I would think that the best way would be to parse in Node, transferring the CSS source to Node instead of parsing in phantomjs. Most likely an issue with regex matching in phantomjs, although I couldn't guarantee that is the problem.