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

Class selector not supported inside :where #88

Closed
brbog opened this issue Aug 8, 2022 · 6 comments
Closed

Class selector not supported inside :where #88

brbog opened this issue Aug 8, 2022 · 6 comments
Assignees

Comments

@brbog
Copy link

brbog commented Aug 8, 2022

After parsing a css file from a Joomla site with the following simplified snippet:
:where(.some-tile:not(.preserve-color))>*{color:#161616}
The logging shows:
LoggingCSSParseExceptionCallback(45) - Failed to parse CSS: [1:8] Encountered text '.' corresponding to token ".". Was expecting one of: ...

So far for the "possible bug" explanation :-). Expecting that the css snippet is correct, I wanted to see if I could produce a test + fix in a pull request. Checking out the repo and building the code works, but my IDE has issues with non-existing classes like com.helger.css.parser.Token and com.helger.css.parser.ParseException. Am I missing something?

If someone wants to jump in on this issue I'm fine with that, but if needed I don't mind helping out as well (if I can get a working project setup in my IDE).

ph-css is now also used inside https://github.com/rzo1/crawler4j and it is already fixing a bunch of issues that the regex alternative would never have been able to do, so thanks!

@phax phax self-assigned this Aug 9, 2022
@phax phax added the bug label Aug 9, 2022
@phax
Copy link
Owner

phax commented Aug 9, 2022

Well, the CSS grammar is constantly evolving, and it seems like the support for :where needs improvement.
Relevant links:

Edit: the parser itself is generated, and you need to add target/generated-classes into your build path, to get the generated classes too

@brbog
Copy link
Author

brbog commented Aug 10, 2022

Thanks, I got the code working and tried putting my test snippet inside a "issue88.css"-file (not sure which directory would be correct though) and at the end of CSSReader30FuncTest.testSpecialCasesAsString() (seemed like the better option as is was easier to debug).

    // Issue 88
    sCSS = ":where(.some-tile:not(.preserve-color))>*{color:#161616}";
    aCSS = CSSReader.readFromStringReader (sCSS, aReaderSettings);
    assertNotNull (aCSS);
    assertEquals (":where(.some-tile:not(.preserve-color))>*{color:#161616}", new CSSWriter (aWriterSettings).getCSSAsString (aCSS));

I'm guessing that the solution lies inside src/main/jjtree/ParserCSS30.jjt file inside void pseudo() : {} where the part:

  | LOOKAHEAD( <FUNCTION> )
    <FUNCTION> { jjtThis.appendText (token.image); }
    ( <S> )*
    ( expr() )?
    <RROUND>  // do not append because of expression!

should look more like:

void negation() : {}
{
  <FUNCTION_NOT> { jjtThis.setText (":not("); }
  ( <S> )*
  ( selector ()
    ( <S> )*
    ( <COMMA>
      ( <S> )*
      selector()
      ( <S> )*
    )*
  )?
  <RROUND>
}

(with the selector () part).
Either that or different extra functions are needed for ":where()", ":is()", and ":has()" which have different semantics than the other pseudo elements like ":hover".

As I'm not really knowledgeable about JavaCC and I suspect this might be a rather more involved fix, I'll probably won't be able to do much more at this point? (sorry)

Update: changed "issue107" to "issue88" (107 was for another project).

@phax
Copy link
Owner

phax commented Aug 12, 2022

Thanks for going so far - I will see what I can do, even though my resources for this are currently quite limited :/

@brbog
Copy link
Author

brbog commented Aug 12, 2022

No problem, that's normal :-).

I was trying to ignore the error (I'm only interested in url content) and played around with:

		final CSSReaderSettings result = new CSSReaderSettings().setCSSVersion(ECSSVersion.LATEST);
		// When parsing fails null is returned by the framework and NullPointerExceptions can arise later on.
		result.setCustomExceptionHandler(new ThrowingCSSParseExceptionCallback());
		// Not needed for our purposes for now:
		// result.setCustomErrorHandler(new ThrowingCSSParseErrorHandler());
		// result.setInterpretErrorHandler();

But only the ExceptionHandler is called in this case (so none of the ErrorHandlers). I expect this sort of "problem handling" to be generated by JavaCC, so I'll probably need to do some preprocessing on failing css files to tweak them in a way that the parser won't choke on its content.

Copy link

stale bot commented Jan 7, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@phax
Copy link
Owner

phax commented Sep 23, 2024

@shagkur sorry for taking so long - I combined the fix with the one for #101 and took a slightly different approach but in general I am very thankful for providing the key ideas on how to solve it. It will be part of the 7.0.3 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants