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

Previous sibling selectors incorrectly parsed without whitespace #69

Open
rockwotj opened this issue Jul 22, 2021 · 12 comments
Open

Previous sibling selectors incorrectly parsed without whitespace #69

rockwotj opened this issue Jul 22, 2021 · 12 comments

Comments

@rockwotj
Copy link
Contributor

rockwotj commented Jul 22, 2021

I have browser compliant mode turned on and the following CSS is incorrectly parsed:

div+p .foo{width:100vw}

But the following works fine

div + p .foo{width:100vw}

But it seems like when you ask for optimized output for div + p .foo{width: 100vw} you get div+p .foo{width:100vw} which means those rules don't roundtrip properly? It seems to handle ~ and > correctly however?

The error message says:

Was expecting one of:

    "*"
    "."
    ":"
    ":not("
    "@bottom-center"
    "@bottom-left"
    "@bottom-left-corner"
    "@bottom-right"
    "@bottom-right-corner"
    "@charset"
    "@footnote"
    "@import"
    "@left-bottom"
    "@left-middle"
    "@left-top"
    "@media"
    "@namespace"
    "@page"
    "@right-bottom"
    "@right-middle"
    "@right-top"
    "@supports"
    "@top-center"
    "@top-left"
    "@top-left-corner"
    "@top-right"
    "@top-right-corner"
    "["
    "|"
    "}"
    <AT_UNKNOWN>
    <FONTFACE_SYM>
    <HASH>
    <IDENT>
    <KEYFRAMES_SYM>
    <PERCENTAGE>
    <S>
    <VIEWPORT_SYM>
@phax phax self-assigned this Jul 24, 2021
@phax phax added the bug label Jul 27, 2021
phax added a commit that referenced this issue Jul 27, 2021
@phax
Copy link
Owner

phax commented Jul 27, 2021

I tried to reproduce your issue but failed.
There is indeed a case where whitespaces need to be around + and - and this is in the context of math operations.
But for this specific case I can't see an issue here - or I might not understand your needs here.... ;-)

@rockwotj
Copy link
Contributor Author

rockwotj commented Jul 28, 2021

Okay I was anonymizing the input and apparently this only repros when the selector is u+b.

so here's one that fails for me: u+b .foo{color:red}
Sorry for botching the test case there 😞

@rockwotj
Copy link
Contributor Author

I wonder if the tokenizer is trying to output a unicode token there?

https://github.com/phax/ph-css/blob/master/ph-css/src/main/jjtree/ParserCSS30.jjt#L315

@phax
Copy link
Owner

phax commented Jul 28, 2021

Hahaha - good catch. I need to check this - interesting....

@rockwotj
Copy link
Contributor Author

Ping - is there anything I can do to help here? FWIW we don't allow for face rules where we do this parsing, so we could drop that token type and it would be fine. Or perhaps that rule needs to move up to the parser level?

@phax
Copy link
Owner

phax commented Sep 15, 2021

Good question - honestly I don't know. This is a very stupid error and requires quite some work in the lexer, because it is very specific to CSS expressions (declaration values). I simply didn't have the time to do this yet.....

@rockwotj
Copy link
Contributor Author

Okay if I find the time I'll try and fix this. I think there may need to be a special lexical state for declarations...

@phax
Copy link
Owner

phax commented Sep 15, 2021

Yes excatly - but the duplicates are quite high - so a lot is the same for "standard" and "declaration values". Lexical states are e.g. already used for comments.

@stale
Copy link

stale bot commented Mar 30, 2022

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.

@stale stale bot added the wontfix label Mar 30, 2022
@phax
Copy link
Owner

phax commented Mar 30, 2022

@rockwotj how high are the chances, you will be able to provide something here?

@stale stale bot removed the wontfix label Mar 30, 2022
@stale stale bot removed the wontfix label Mar 30, 2022
@rockwotj
Copy link
Contributor Author

Not anytime soon unfortunately 😔

@phax
Copy link
Owner

phax commented Mar 30, 2022

Thanks for the swift response - levaing this one open for now

@phax phax added the pinned label Mar 30, 2022
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