Skip to content

CSS: Don't special-case backslashes when trimming CSS Custom Properties #4938

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
wants to merge 1 commit into from

Conversation

mgol
Copy link
Member

@mgol mgol commented Sep 24, 2021

Summary

The original logic from PR gh-4930 re-used the rtrim regex from the selector
module but selectors have special escaping rules which means whitespace
following a backslash character would not get trimmed.

This change splits the logic at the unfortunate but necessary cost of size.

Ref gh-4926
Ref gh-4930

This is at +44 bytes now but together with gh-4930 where it should be included it's at about half that.

When this lands, I'll CP gh-4930 together with this PR to 3.x-stable.

Checklist

Sorry, something went wrong.

The original logic from PR jquerygh-4930 re-used the `rtrim` regex from the selector
module but selectors have special escaping rules which means whitespace
following a backslash character would not get trimmed.

This change splits the logic at the unfortunate but necessary cost of size.

Ref jquerygh-4926
Ref jquerygh-4930
@mgol mgol added CSS Needs review Blocker Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Sep 24, 2021
@mgol mgol added this to the 3.6.1 milestone Sep 24, 2021
@mgol mgol requested review from timmywil and gibson042 September 24, 2021 12:18
@mgol mgol self-assigned this Sep 24, 2021
@mgol
Copy link
Member Author

mgol commented Sep 27, 2021

As discussed at the today's team meeting, this is not a correct change. Whitespace is getting trimmed after tokenization and some tokens contain a trailing whitespace.

We still need to discuss whether the current solution from #4930 is fine or if it needs more iterations but this PR is not what we need.

@mgol mgol closed this Sep 27, 2021
@mgol mgol removed this from the 3.6.1 milestone Sep 27, 2021
@mgol mgol removed Needs review Blocker Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

1 participant