Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Added rule to detect multiple usage of the same background-image #200

Merged
merged 2 commits into from

3 participants

Hans-Peter Buniat Nicholas C. Zakas Nicole Sullivan
Hans-Peter Buniat

When working with sprites it is likely that someone writes css like:
.aSpriteImage {background: url(image.png) ..}
.anotherSpriteImage {background: url(image.png) ..}
.yetAnotherSpriteImage {background: url(image.png) ..}

The common background-image should be applied by using a common class or specific selectors. This rule should detect such issues.

Nicholas C. Zakas

It would be better to do a reporter.report() at the line of the second instance of the image rather than a rollup warn. We're trying to get away from rollups.

Nicholas C. Zakas

Instead of going through this complex regular expression, which might miss multiple URLs on the same line, it would be better to examine each of the value parts. The value.parts array has each part of the value, and you can iterate over those parts and check the type property to see if it's a URI. For example:

if (value.parts[i].type == "uri") {
   actualUri = value.parts[i].uri;
}

All of the URL parsing is already done for you, you just need to grab the info.

Nicholas C. Zakas
Owner

Thanks for the pull request. This seems like a good idea. I've added some notes to the commit. If you can make those changes, I can merge in.

Hans-Peter Buniat

Thanks for your comments. I've updated the pull-request.

Nicole Sullivan

I think people might find this desc confusing and take it to mean "don't use sprites". Can we clarify it?

Nicole Sullivan

I like this rule, but I'm concerned that people will take it to mean "don't use sprites" rather than "abstract your sprite class name". What should the error message look like to make the distinction very clear?

Hans-Peter Buniat

Most likely, the issue will occur while working with sprites - but there might also be arbitrary c&p. I think "Always use abstract class names for sprites, which should at least declare the background-image" brings it to the point - in almost all cases.
As this also violates principles of OOCSS, the rule might also refer to it?

Nicholas C. Zakas
Owner

I think this fits under the "Performance" category quite nicely (reducing bytes) as "Disallow duplicate images". @hpbuniat - I'll merge this in, do you want to take a stab at writing up some documentation in this form: https://github.com/stubbornella/csslint/wiki/Beware-of-box-model-size

Nicholas C. Zakas nzakas merged commit b2ac3b7 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
37 src/rules/duplicate-background-url.js
View
@@ -0,0 +1,37 @@
+/*
+ * Rule: Disallow duplicate background-images (using url).
+ */
+/*global CSSLint*/
+CSSLint.addRule({
+
+ //rule information
+ id: "duplicate-background-url",
+ name: "Disallow duplicate background-url",
+ desc: "Every background-image should be unique. Use a common class for e.g. sprites.",
+ browsers: "All",
+
+ //initialization
+ init: function(parser, reporter){
+ var rule = this,
+ stack = {};
+
+ parser.addListener("property", function(event){
+ var name = event.property.text,
+ value = event.value,
+ i;
+
+ if (name.match(/background/i)) {
+ for (i in value.parts) {
+ if (value.parts[i].type == 'uri') {
+ if (typeof stack[value.parts[i].uri] === 'undefined') {
+ stack[value.parts[i].uri] = event;
+ }
+ else {
+ reporter.report("Background-Image (" + value.parts[i].uri + ") was used multiple times, first declared at line " + stack[value.parts[i].uri].line + ", col " + + stack[value.parts[i].uri].col + ".", event.line, event.col, rule);
+ }
+ }
+ }
+ }
+ });
+ }
+});
25 tests/rules/duplicate-background-url.js
View
@@ -0,0 +1,25 @@
+(function(){
+
+ /*global YUITest, CSSLint*/
+ var Assert = YUITest.Assert;
+
+ YUITest.TestRunner.add(new YUITest.TestCase({
+
+ name: "Duplicate Background-URL Rule Errors",
+
+ "duplicate background-image should result in a warning": function(){
+ var result = CSSLint.verify(".foo { background-image: url('mega-sprite.png'); } .foofoo { background-image: url('fancy-sprite.png'); } .bar { background-image: url(\"mega-sprite.png\"); } .foobar { background: white url(mega-sprite.png); }", {"duplicate-background-url": 1 });
+ Assert.areEqual(2, result.messages.length);
+ Assert.areEqual("warning", result.messages[0].type);
+ Assert.areEqual("Background-Image (mega-sprite.png) was used multiple times, first declared at line 1, col 8.", result.messages[0].message);
+ },
+
+ "duplicate background with url should result in a warning": function(){
+ var result = CSSLint.verify(".foo { background: url(mega-sprite.png) repeat-x; } .foofoo { background-image: url('fancy-sprite.png'); } .bar { background: white url(\"mega-sprite.png\") no-repeat left top; } .foobar { background: white url('mega-sprite.png'); }", {"duplicate-background-url": 1 });
+ Assert.areEqual(2, result.messages.length);
+ Assert.areEqual("warning", result.messages[0].type);
+ Assert.areEqual("Background-Image (mega-sprite.png) was used multiple times, first declared at line 1, col 8.", result.messages[0].message);
+ }
+ }));
+
+})();
Something went wrong with that request. Please try again.