Skip to content

Parser ignores strings at an unknown at-rule prelude (thus failing at { and ; there) #3147

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

Closed
sylingd opened this issue Jan 1, 2018 · 11 comments

Comments

@sylingd
Copy link

sylingd commented Jan 1, 2018

I used less to write some user styles for Stylish, it have some special syntaxs, for example, this extension support regexp("Regular expression"). So the following codes will work well on Stylish:

@-moz-document regexp("(\d{0,15})") {
	a {
		color: red;
	}
}

But it can not work with less.js:
tim 20180102004110

The less.js says:

ParseError: Unrecognised input. Possibly missing opening '{' on line 1, column 32:

1 @-moz-document regexp("(\d{0,15})") {
2 	a {

Is there a way to ignore the characters between quotation marks? Or are there other ways to solve this problem?
Thank you

@rjgotten
Copy link
Contributor

You can insert literal content using a string that is 'unquoted' via the ~ operator.
What might work for you is something like:

@-moz-document regexp(~'"(\d{0,15})"') {
  a {
    color : red;
  }
}

If that still messes with the parser, you may consider moving the expression contents into a variable first:

@regexp : ~'"(\d{0,15})"';
@-moz-document regexp(@regexp) {
  a {
    color : red;
  }
}

@seven-phases-max
Copy link
Member

seven-phases-max commented Feb 15, 2018

Apparently the problem is in "{}" part (i.e. @-moz-document regexp("{}") ... fails as well) - and that's a bug (we'd expect anything within quotes to be parsed fine).
Then yes, the workaround would be in moving it to a separate variable as in the second @rjgotten example. - Oh, no, it woud not - see below.

P.S. Recalling how custom at-rules are parsed, I guess the problem here is that the parser does not even triy to parse anything between @-moz-document and first { as something meaningful (i.e. quotes has no expected effect here at all), and this is sort of expected since the parser has absolutely no idea of what syntax this custom at-rule may have... Thus it may be quite tricky to fix.

For the same reason ("unknown grammar") the workaround with @regexp won't work simply because it won't mean a variable there (in general nothing after @unknown-at-rule and before the first { is considered to be a Less code).
I.e. this construction is essentially parsed as:

@-moz-document regexp("(\d {
    0,15})") {
        a {
            color: red;
        }
    }

And the only workarounds I can invent at quick glance in this case would be either a separate file injected with (inline) or something awful like:

@mzd: ~'-{} @-moz-document regexp("(\d{0,15})")';

@keyframes @mzd {
    a {
        color: red;
    }
}

or sort of.

@rjgotten
Copy link
Contributor

rjgotten commented Feb 15, 2018

@seven-phases-max
Crap; you're right. I forgot about the (by necessity) rather ignorant / non-existent parsing logic for at-rules.

In that case, I wonder if Less can do any better in the look-ahead code to match the opening brace of the at-rule's block. E.g. make some assumptions on sanity of the structure of the value(s) that follow the at-directive, and performing some rudimentary open/close-matching for brace/bracket/quote characters on them.

@seven-phases-max
Copy link
Member

seven-phases-max commented Feb 15, 2018

In that case, I wonder if Less can do any better in the look-ahead code to match the opening brace of the at-rule's block.

I think it can try to parse that at-rule-prelude thing as an array of tokens (at least of two types: "string" and "anything-else", parens pairs may probably be ignored) instead of just a single "whatever" token like it does now... That should do the trick, though it's hard to predict what other problems may arise when you dig into.

@seven-phases-max seven-phases-max changed the title Question: How to ignore characters between the quotation marks Parser ignores strings at an unknown at-rule prelude (thus failing at { and ;) Feb 15, 2018
@seven-phases-max seven-phases-max changed the title Parser ignores strings at an unknown at-rule prelude (thus failing at { and ;) Parser ignores strings at an unknown at-rule prelude (thus failing at { and ; there) Feb 15, 2018
@matthew-dean
Copy link
Member

@seven-phases-max (or anyone)
Hey, if you're looking at parsing and dealing with permissiveness, do you think you would have time to look at #2715 & #2986? I just added this comment.

@matthew-dean
Copy link
Member

@rjgotten

In that case, I wonder if Less can do any better in the look-ahead code to match the opening brace of the at-rule's block. E.g. make some assumptions on sanity of the structure of the value(s) that follow the at-directive, and performing some rudimentary open/close-matching for brace/bracket/quote characters on them.

It sounds to me like the rules for at-rules and custom property syntax may be similar. That is, you can include almost anything for an unknown at-rule, but there are some pair-matching rules? It might make sense to see what the overlap is and normalize the parsing between them.

@rjgotten
Copy link
Contributor

@matthew-dean

Yes; that's the conclusion I arrived at as well. ^_^
There's got to be some overlap of concerns there.

@ghost
Copy link

ghost commented Apr 25, 2018

The Stylus extension (not to be confused with the Stylus language) (also used for applying userstyles, a fork of Stylish) recently added Less support, this bug however greatly limits its use - especially since the file workaround cannot be used.
It'd be awesome if a fix for this was to be developed soonish so we can make use of Less. :)

@tophf
Copy link

tophf commented Apr 25, 2018

The spec says {-token, not {-character. The parser works by consuming/ignoring tokens. The parser should read and ignore tokens until it encounters {-token, in other words: ident-token regexp, (-token which starts a simple block i.e. it'll consume everything (even { or } and the doublequoted string inside) until the closing )-token. Only then it'll see {-token, and then it'll consume the {} block. Less seems to simply search for { character which is plain wrong.

@seven-phases-max
Copy link
Member

seven-phases-max commented Apr 25, 2018

The parser works by consuming/ignoring tokens.

Less parser does not. I.e. you are right about how it should be in a perfect world (above I wrote the same) but I'm afraid how Less parser works internally has nothing to do with CSS parsing specs.

@matthew-dean
Copy link
Member

So I thought I'd do some tests in Chrome as to what are considered valid custom properties by the CSS custom property parser:

TL;DR - The theory of what's valid for custom properties (in Chrome anyway) mostly matches the way the spec seems to read, with Chrome seeming to allow for unmatched braces in some cases (probably error correction?).

Results:
--test: { blah; } - valid
--test: { blah; - interestingly, valid
--test: () { blah; } - valid
--test: () => { blah; } - valid
--test: () => { blah; blah; } - valid
--test: () => { blah; blah blah blah; } - valid
--test: () => { blah; blah blah blah; [ } - not valid
--test: () => { blah; blah blah blah; [blah] } - valid (confirms "matching brace" spec)
--test: () => { blah; blah blah blah; ( } - not valid
--test: () => { blah; blah blah blah; (blah) } - valid
--test: () => { blah; blah blah blah; { } - bizarrely, valid
--test: () => { blah; blah blah blah; {} } - valid
--test: () => { blah; blah blah blah; {}}} - not valid
--test: () => { blah; blah blah blah;'; - valid
--test: () => { blah; blah blah blah;"; - valid
--test: () => { basically anything until final semi-colon; even other stuff; } - valid
--test: () => { basically anything until final semi-colon; even other stuff; // i'm serious } - not valid
() => { basically anything until final semi-colon; even other stuff; // im serious } - valid (with apostrophe removed, which in this case, invalidated?)
--test: () => { basically anything until final semi-colon; even other stuff; // i\'m serious } - valid (is this in the spec? escaping?)
--test: () => { basically anything until final semi-colon; even other stuff; // i'm serious' } - valid - apostrophe seems to be treated as "matching quote"

matthew-dean added a commit that referenced this issue Jun 22, 2018
* Adds permissive parsing for at-rules and custom properties
* Added error tests for permissive parsing
* Change custom property value to quoted-like value
* Allow interpolation in unknown at-rules
* Allows variables to fallback to permissive parsing
* Allow escaping of blocks
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

5 participants