Invalid rules cause valid rules to be discarded

[hrj]

One of the CSS2.1 test-suite has the following style code:

   #inner { background: white;
               border: 20px solid;
               border-color: #FCFCFC; #FCFCFC; #FDFDFD #FCFCFC;
               }

jStyleParser discards the whole rule, whereas other browsers recognize all three properties.

[radkovo]

Thanks for reporting this. Since jstyleparser2 (ANTLR4 based) seems to be quite stable now, I would like to make it a new master and fix new issues there. If you are able to run your regression tests with the jstyleparser2 branch, I would be grateful for any feedback.

[hrj]

@radkovo I have started running the tests. Quick feedback: works mostly fine, though there are some regressions around @ rules.

I also encountered an exception while parsing this CSS:

java.lang.NullPointerException
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitTerms(CSSParserVisitorImpl.java:806)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitFunct(CSSParserVisitorImpl.java:866)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitValuepart(CSSParserVisitorImpl.java:939)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitTermValuePart(CSSParserVisitorImpl.java:843)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitTerms(CSSParserVisitorImpl.java:808)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitDeclaration(CSSParserVisitorImpl.java:719)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitDeclarations(CSSParserVisitorImpl.java:680)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitRuleset(CSSParserVisitorImpl.java:657)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitStatement(CSSParserVisitorImpl.java:273)
    at cz.vutbr.web.csskit.antlr4.CSSParserVisitorImpl.visitStylesheet(CSSParserVisitorImpl.java:232)
    at cz.vutbr.web.csskit.antlr4.CSSParserFactory.parse(CSSParserFactory.java:206)
    at cz.vutbr.web.csskit.antlr4.CSSParserFactory.parseAndImport(CSSParserFactory.java:147)
    at cz.vutbr.web.csskit.antlr4.CSSParserFactory.parse(CSSParserFactory.java:72)
    at cz.vutbr.web.csskit.antlr4.CSSParserFactory.parse(CSSParserFactory.java:93)

Will give a detailed report soon.

[hrj]

The jstyleparser2 branch is definitely better at recovering from CSS errors. There are some regressions though:

• uri-007.xht
• uri-003.xht
• font-family-invalid-characters-001.xht
• comments-003.xht
• at-rule-005.xht
• at-charset-052.xht

Explaining each one will be tedious. Instead, I will give you a guide to reproduce the failures.

• Download the test-suite by cloning this repo.
• Run python -m SimpleHTTPServer in the directory.
• You can then navigate to http://localhost:8000/nightly-unstable/xhtml1/<testname>.

[radkovo]

The originally reported issue should be fixed by this commit. I will take a look at the remaining regressions asap.

[hrj]

I tried the regressions tests again, using commit c7525aa3ef4ffba88f1d6f7fab43f02bf6018931

I confirm that the original issue reported here is fixed. Further, only 4 regressions remain:

• font-family-invalid-characters-001.xht
• comments-003.xht
• at-rule-005.xht
• at-charset-052.xht

cheers!

[radkovo]

Ok, the last regression at-rule-005 should be fixed by 08eda6fa97cf9709aa64b653b747fd93d12dcd0a. Thanks for the feedback. Many tests from the test suite are still failing, I will keep fixing them bit by bit. Anyway, jstyleparser2 should work better than jstyleparser1 now. Regarding performance: there is a lot of debugging output being produced by the parser now. Once this output is removed, jstyleparser2 should be faster than jstyleparser1 (I believe).

[hrj]

Confirmed; there are no regressions now, and a total of 19 progressions. :clap:

Please let me know when jstyleparser2 branch is ready for production. I can run our tests once again before you merge to master.

[radkovo]

I have made some last fixes in jstyleparser2. Now, I would like to take a few days off so if you have time, it is a good opportunity to run any tests. If everything is ok, I will merge jstyleparser2 to master and I will continue the work there. Thanks for the feedback!

[hrj]

Ran the tests; looks good :+1:

:shipit:

[hrj]

@radkovo Any news about merging to master? Would like to roll out a new release for gngr, and it "feels good" to use master branch :)

[radkovo]

@hrj I am sorry for the delay, I have been traveling a bit. It should be merged now. If everything is fine, I will make a 2.0 release in a few days.

[hrj]

@radkovo Thanks!