Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Broken CSS if contains classes starting with dash #993

Open
f-liva opened this issue Jul 15, 2015 · 5 comments
Open

Broken CSS if contains classes starting with dash #993

f-liva opened this issue Jul 15, 2015 · 5 comments

Comments

@f-liva
Copy link

f-liva commented Jul 15, 2015

Target: <div class="block__element --modifier"></div>

Original CSS:

.block__element.\--modifier {
    outline: 1px solid red,
}

Standard CSS minified output:

.block__element.\--modifier{outline:1px solid red}

PageSpeed CSS minified output:

.block__element.--modifier{outline:1px solid red}

PageSpeed it removes the backslash before the dash and break all stylesheet.

@jeffkaufman
Copy link
Contributor

Looking at the css spec I don't think --modifier is legal here. A class is an ident, which is -?{nmstart}{nmchar}*. That means "an optional -, followed by one of [_a-z], {nonascii}, or {escape}, followed by any combination of zero or more of [_a-z0-9-], {nonascii}, or, {escape}". In other words, classes are specifically banned from starting with -- because {nmstart} doesn't include -. And note that {escape} is defined as "either {unicode} or \\[^\r\n\f0-9a-f]", which also does not allow \-.

Are you sure this works in all browsers?

(If there's something people are doing in html/css/js that isn't to spec but is commonly used and widely supported by browsers then we should fix our minifier to work with it to avoid breaking sites.)

@f-liva
Copy link
Author

f-liva commented Jul 16, 2015

Thank you for the exhaustive answer. Yes, these class names work in every browser, even the old and dated IE 7 and Safari 5. Could you consider to implement the correct handling of these class names?

@jeffkaufman
Copy link
Contributor

Ok, sounds like this is browser behavior we have to work with.

@jmarantz
Copy link
Contributor

If our CSS parser considers CSS to be invalid, it should not minify it and
lose content. It can also black-box the lexical region with the error but
minify everything else around it. Further, even in syntactically invalid
CSS we have a fallback parser than can do whatever is needed to embedded
URLs (e.g. absolutify them if the CSS itself is being moved).

What actually happens to the CSS in this case? If we are actually breaking
it in some way then that's a bug.

-Josh

On Thu, Jul 16, 2015 at 9:21 AM, Jeff Kaufman [email protected]
wrote:

Ok, sounds like this is browser behavior we have to work with.


Reply to this email directly or view it on GitHub
#993 (comment)
.

@f-liva
Copy link
Author

f-liva commented Jul 24, 2015

The PageSpeed outputted CSS contains the first --class occurrence correctly escaped with .\--class and the next occurrences not escaped, and these brake the stylesheet.

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

No branches or pull requests

3 participants