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

Incorrect ordering / Inconsistent css rule merging #267

Closed
altmind opened this issue Mar 27, 2014 · 4 comments
Closed

Incorrect ordering / Inconsistent css rule merging #267

altmind opened this issue Mar 27, 2014 · 4 comments

Comments

@altmind
Copy link

altmind commented Mar 27, 2014

Using standalone cleancss-cli 2.1.7.

Processing

.nav-item-link {
    height: 7rem; /* DECLARED FIRST */
}

.nav-item-link-product {
    height: auto; /* DECLARED SECOND */
}

.nav-item-link {
    line-height: 7rem; /* BREAKS ORDERING*/
    color: red;
}

results in

.nav-item-link-product{height:auto}
.nav-item-link{height:7rem;line-height:7rem;color:red}

which is semantically wrong - now .nav-item-link will overwrite height for html element with class="nav-item-link-product nav-item-link". Clear problem with css rule priorities.

I'm puzzled, as when we remove "line-height: 7rem;" from 3rd rule, we get correct result css. See:

.nav-item-link {
    height: 7rem; /* DECLARED FIRST */
}

.nav-item-link-product {
    height: auto; /* DECLARED SECOND */
}

.nav-item-link {
    color: red;
}

generates

.nav-item-link{height:7rem}
.nav-item-link-product{height:auto}
.nav-item-link{color:red}

So it seems that 'color' does not cause .nav-item-link rules merge, whereas 'line-height' property does cause rules merge.

When using option "noAdvanced" clean-css generates correct result.

This problem was observed on production and is real, practical, issue

@GoalSmashers
Copy link
Contributor

Seems like an edge case in our selectors merging. Thanks for detailed report, fix is coming!

@GoalSmashers
Copy link
Contributor

The fix is not as straightforward as I thought, so, if possible, use noAdvanced until we fix it later today.

@GoalSmashers
Copy link
Contributor

@altmind 2.1.8 is out with the fix!

@altmind
Copy link
Author

altmind commented Mar 31, 2014

👍

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

No branches or pull requests

2 participants