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

Allow numbers and underscores in attribute selectors #879

Merged
merged 2 commits into from
Jul 27, 2012
Merged

Allow numbers and underscores in attribute selectors #879

merged 2 commits into from
Jul 27, 2012

Conversation

dmcass
Copy link
Contributor

@dmcass dmcass commented Jul 26, 2012

This PR incorporates the fix from PR #876 and adds the fix for #863, and also adds a test case that covers both.

@Synchro
Copy link
Member

Synchro commented Jul 26, 2012

Though this does fix the bug as reported, I agree with other comments in #876 that we should accept anything that's valid, which is quite a bit broader than this regex. Can save that for another day though.

@dmcass
Copy link
Contributor Author

dmcass commented Jul 27, 2012

Added a commit to make character classes in regular expressions consistent across the parser.js file. I'll work on extending allowable characters when I have time, but that's a much bigger job considering that you have to account for escapable characters, most UTF8 characters, etc. I'm not sure it's within the scope of this pull request.

@dmcass
Copy link
Contributor Author

dmcass commented Jul 27, 2012

So I put a little time into extending support to all valid attribute selectors, and it makes me question how necessary it really is because its complexity is absurd. The really crazy part is that this doesn't even support the full character set (can't use unicode characters higher than \uffff in JS).

The regex which will match all valid attribute selectors (any valid ident token from the CSS syntax rules):
/^-?(?:[_A-Za-z\u0080-\uD7FF\uE000-\uFFFD]|\\[A-Fa-f0-9]{1,6}|\\[\u0020-\u007E\u0080-\uD7FF\uE000-\uFFFD])(?:[_A-Za-z\u0080-\uD7FF\uE000-\uFFFD-]|\\[A-Fa-f0-9]{1,6}|\\[\u0020-\u007E\u0080-\uD7FF\uE000-\uFFFD])*/

So, the question is, does the parser need to be 100% complete in its support of CSS, or only support the majority of use cases? @agatronic, @MatthewDL, @cloudhead, or anyone else who is actively maintaining: can I get your input here?

Synchro added a commit that referenced this pull request Jul 27, 2012
Allow numbers and underscores in attribute selectors
@Synchro Synchro merged commit 65e4806 into less:master Jul 27, 2012
@Synchro
Copy link
Member

Synchro commented Jul 27, 2012

Par for the course for anything from the W3C! I agree that it's outside the scope for this issue, which is what I meant about being able to wait. While it's in the spirit of 'be liberal in what you accept and strict in what you emit', in practice it's unlikely to be needed any time soon, and the regex you wrote will come in handy if it does get revisited. Thanks for cleaning up all the regexes.

@zackd
Copy link

zackd commented Aug 23, 2012

would desperately love a new release with these fixes! :)

thanks for the awesome lib!

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

Successfully merging this pull request may close these issues.

None yet

3 participants