When images are used twice, the URI is generated twice. This inflates the size of the newly generated CSS file.

div.1 {background: url('img.png')}

div.2 {background: url('icon.png')}

div.3  {background: url('img.png')}

One solution would be to merge the selectors with the same images before processing the images to URI.

The above example should become:

div.2 {background: url('icon.png')}

div.1, div.3  {background: url('img.png')}

Comments

philbar’s picture

I'm assuming this is a issue based on the conversation here: http://drupal.org/node/869304#comment-3285472

My main focus lately has been on dealing with the last point, including a data URI only once, but it tends to cause problems with complex themes because it changes the cascade order (as does separating the stylesheets), especially when dealing with IE's 32Kb limit.

I have not personally encountered this problem.

jcarnett’s picture

Status: Active » Closed (works as designed)

The current 2.x code will merge declarations where possible. However, it will not change the order of the declarations as in this example. Doing so risks altering the cascade order and it's not possible to safely do that without knowing how the styles are used in the document (something we can't know).

Take your example CSS above and apply it to the following markup:

<div class="1 2">Hello</div>

In the original CSS the div background is "icon.png." After merging the declarations, the background is "img.png."

What the module will currently do is merge consecutive declarations using the same image. While this is not a particularly common case, it can help a little bit. I'm not really convinced it's possible to do better so my current recommendation is to minimize duplicate image references.

philbar’s picture

Status: Closed (works as designed) » Active

Using the same scenario, we could run into the following problem. The current output by the module would be something like this:

div.1 {background: url(data:image/gif;base64,
  R0lGODlhCgAKAMQUAM/Q0vT09Ojr7s7Q0dLU1f7+/u/z9+Dj5tfX1+Dg4MrKyu
  fq7tfZ27m5uba2tsLCwvv7+7W1tbS0tP////P3+wAAAAAAAAAAAAAAAAAAAAAA
  AAAAAAAAAAAAAAAAAAAAACH5BAEAABQALAAAAAAKAAoAAAU0ICWOZGkCkaRKEU
  AtTzHNU/Es1JDQUzKIggZkBmkIRgTEDEEgGRyBgMNQYigUDBPlcCCFAAA7)}

div.2 {background: url(data:image/gif;base64,
  RFI2J09F0I390F0V0I303RF09J3RF09J3R0JF09J30RJF0J0JR03J0)}

div.3  {background: url(data:image/gif;base64,
  R0lGODlhCgAKAMQUAM/Q0vT09Ojr7s7Q0dLU1f7+/u/z9+Dj5tfX1+Dg4MrKyu
  fq7tfZ27m5uba2tsLCwvv7+7W1tbS0tP////P3+wAAAAAAAAAAAAAAAAAAAAAA
  AAAAAAAAAAAAAAAAAAAAACH5BAEAABQALAAAAAAKAAoAAAU0ICWOZGkCkaRKEU
  AtTzHNU/Es1JDQUzKIggZkBmkIRgTEDEEgGRyBgMNQYigUDBPlcCCFAAA7)}

If the duplicate images takes longer to download (a duplicate 32kb image) than the added DNS lookups, then this module becomes greatly crippled as an optimization technique.

I think it's time to consider an "exclusion list" for such cases when this module breaks CSS or doesn't optimize the CSS properly.

With the possibility to exclude problem image conversions, you can merge selectors without fear of messing up layout because the problem images can be ignored by the module.

Ideally, it would be great to have a settings section list out all the CSS images with a checkbox next to each so site builders can scan down the list and exclude the problem images, or images they choose to exclude such as backend images. But that will likely take quite a bit of work.

jcarnett’s picture

I don't think it's necessary to ask site builders to select images to include/exclude as they might not even know when it's a good idea to do so and it should be something we can automate fairly well. We can easily determine if an image is referenced multiple times in the CSS then make a decision of whether to embed it or leave it external based on its size. If the extra size added from embedding the image multiple times is less than the cost of another RTT, then it should be embedded multiple times; if not, it should be external. Google's Page Speed recommendations are a good place to start in determining where that switch happens -- quickly ballparking I'd probably say 10KB is a good threshold.

For example, say we have a 2KB image referenced three times in the CSS. The total cost of embedding it three times is (2KB * 3 * 1.333) or just below 8KB. If it's faster to download 8KB than to make another round trip requesting a 2KB file, then it makes sense to embed the image three times.

A similar approach can be applied to the 2.x feature that puts the image data in a separate CSS file. If that file is going to be small, we might as well just take the 1.x approach and embed the image data directly in the original CSS instead of a separate file. In this case we have to consider both RTT and the rendering delay added by the extra data, but I suspect it's beneficial below a certain threshold.

I'll have to investigate both of these a bit more to come up with those thresholds, but this seems like the best and most usable approach.

jcarnett’s picture

Status: Active » Fixed

I've implemented the above and committed it to CVS for 6.x-2.x and 7.x-1.x. I went with a 10KB threshold for now, so if embedding a duplicate image would result in more than 10KB of data the image will not be embedded. This is based on the assumption that most site visitors have a fairly high downstream/upstream ratio and a fairly low latency connection, which is probably a safe bet these days. I didn't add a config form for it, but this limit is configurable by setting the 'css_emimage_duplicate_embed_limit' variable manually. For example, you might want to increase it if your site visitors tend to have higher latency connections.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.