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

Improve property merging algorithm #168

Closed
GoalSmashers opened this issue Nov 3, 2013 · 16 comments
Closed

Improve property merging algorithm #168

GoalSmashers opened this issue Nov 3, 2013 · 16 comments
Assignees
Milestone

Comments

@GoalSmashers
Copy link
Contributor

a{color:white;color:black;color:red;}

does not get minified into

a{color:red}

That's because we assume adjoining properties to be used deliberately, e.g.:

a {
  color:rgba(0,0,0,0.5);
  color:#888;
}

or

a{
  background:#fff;
  background:-moz-linear-gradient(...);
  background:-webkit-linear-gradient(...);
  background:linear-gradient(...);
}
@ghost ghost assigned GoalSmashers Nov 3, 2013
@XhmikosR
Copy link
Contributor

XhmikosR commented Nov 3, 2013

Refs e25a329#commitcomment-4496293

@GoalSmashers
Copy link
Contributor Author

One way would be to scan for vendor prefixes and functions. This would include cases like display:-moz-inline-box, color:rgba(...) and background:linear-gradient(...).

@Venemo
Copy link
Contributor

Venemo commented Nov 19, 2013

In your example, if background:#fff comes after the other definitions, it will override them.
For example, in foo{color:rgba(0,0,0,1);color:#fff} the color:#fff wins even in browsers that support rgba(0,0,0,1) - we only have to worry about the cases when rgba(...) comes after #...

@Venemo
Copy link
Contributor

Venemo commented Nov 19, 2013

NOTE: It seems that in IE, this fallback technique doesn't always work, see this SO question. :( There is a hacky workaround, though.

@XhmikosR
Copy link
Contributor

The fallback color is supposed to be a fallback one if it comes before rgba... Otherwise it's not our job to fix it in my opinion.

Clean-css should do what the browsers do; ignore the previous declaration regardless of what it is. This behavior is what the person who wrote the code expects, even though it's wrong.

@GoalSmashers
Copy link
Contributor Author

@Venemo - correct, background:#fff should come first. It's fixed.

@Venemo, @XhmikosR - indeed we should not be smarter than the authors about their intents. If it fails in the browser then it is not our concern.

@Venemo
Copy link
Contributor

Venemo commented Nov 19, 2013

@GoalSmashers Same goes for color. :)

@Venemo
Copy link
Contributor

Venemo commented Nov 20, 2013

I've implemented a logic for this in my fork.

The idea is the following: "more understandable" properties always override "less understandable" properties but not the other way around. This ensures that the result will be optimal but will also keep any fallbacks where feasible.

Some examples

foo{color:red;color:rgba(1,2,3,0.5);color:#fff}
/* is transformed into */
foo{color:#fff}

/* but */

foo{color:red;color:#fff;color:rgba(1,2,3,0.5)}
/* is transformed into */
foo{color:#fff;color:rgba(1,2,3,0.5)}

/* combined with shorthand compaction */

foo{background-color:#fff;background:url(aaa);background-color:rgba(1,2,3,0.5)}
/* is transformed into */
foo{background:rgba(1,2,3,0.5) url(aaa)}
/* because background will clear out the preceding background-color anyway */

For colors, I define understandability like this: (named|hex) > rgb > (rgba|hsl|hsla)

For background image: none > url > anything else > none (so that none both overrides anything and is overridden by anything)

@GoalSmashers
Copy link
Contributor Author

That's the way it should work. Well done 💯

@Venemo
Copy link
Contributor

Venemo commented Nov 20, 2013

@GoalSmashers I was unaware of this, but whenever there's a URL in the input, I get something like __ESCAPED_URL_CLEAN_CSS0__ inside the property optimizer.

I have two questions:

  • Can I safely assume that anything that begins with __ESCAPED_URL_CLEAN_CSS is a placeholder for a URL?
  • Are there any other similar mechanisms?

@GoalSmashers
Copy link
Contributor Author

Actually any free text is escaped this way to avoid processing via regexps (so comments, URLs, content:'...', selectors like [data-json='…']). See all modules under lib/text.

So it is safe to make that assumption.

@Venemo
Copy link
Contributor

Venemo commented Nov 20, 2013

Awesome, thanks!

@tomByrer
Copy link
Contributor

foo{background-color:#fff;background:url(aaa);background-color:rgba(1,2,3,0.5)}
/* is transformed into */
foo{background:rgba(1,2,3,0.5) url(aaa)}

I would not want this, since I need similar behavior for IE8 fallback unfortunately.
Neat idea, & should be added since many are targeting IE10+ now... Perhaps behind a flag, with the default off until IE8 dies? I personally would be OK with default on, but have a comment-flag that will preserve a section of code, however others may not like the nasty surprise if they don't read every inch of docs.

@Venemo
Copy link
Contributor

Venemo commented Jan 29, 2014

@tomByrer Even in IE8 or older browsers, the background will clear out the preceding background-color. If you want fallback, use the following syntax.

foo{background:#fff url(aaa);background-color:rgba(1,2,3,0.5)}

That is, put the "less understandable" syntax after the "more understandable" one: in this case, it will be compatible with older browsers and my property optimizer will not remove it either.

@tomByrer
Copy link
Contributor

That's true, ty @Venemo

@GoalSmashers
Copy link
Contributor Author

Fixed with #249.

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

4 participants