Change the way offenders are displayed for some CSS related rules

front/src/views/rule.html

@@ -30,7 +30,7 @@
         <p>This rule reached the abnormality threshold, which means there is a real problem you should care about.</p>
     </div>
     <div class="offenders" ng-if="rule.policy.hasOffenders">
-        <h3><ng-pluralize count="rule.offendersObj.count" when="{'0': 'No offenders', 'one': '1 offender', 'other': '{} offenders'}"></ng-pluralize></h3>
+        <h3 ng-if="rule.offendersObj.count >= 0"><ng-pluralize count="rule.offendersObj.count" when="{'0': 'No offenders', 'one': '1 offender', 'other': '{} offenders'}"></ng-pluralize></h3>
 
         <div ng-if="rule.offendersObj.list" class="offendersTable">
             <div ng-repeat="offender in rule.offendersObj.list track by $index">
@@ -106,20 +106,14 @@
                         <div class="similarColors checker"><div ng-style="{'background-color': offender.color1, 'color': offender.isDark ? '#FFF' : '#000'}">{{offender.color1}}</div><div ng-style="{'background-color': offender.color2, 'color': offender.isDark ? '#FFF' : '#000'}">{{offender.color2}}</div></div>
                     </div>
 
-                    <div ng-if="policyName === 'cssMobileFirst'">
-                        <b>{{offender.query}}</b> for <ng-pluralize count="offender.rules" when="{'one':'1 rule','other':'{} rules'}"></ng-pluralize>
-                        <file-and-line-button file="offender.file" line="offender.line" column="offender.column"></file-and-line-button>
-                    </div>
-
                     <div ng-if="policyName === 'cssParsingErrors'">
                         <b>{{offender.error}}</b>
                         <file-and-line file="offender.file" line="offender.line" column="offender.column"></file-and-line>
                         <span ng-if="offender.file">(<a href="http://jigsaw.w3.org/css-validator/validator?profile=css3&usermedium=all&warning=no&uri={{offender.file | encodeURIComponent}}" target="_blank">Check on the W3C validator</a>)</span>
                     </div>
 
-                    <div ng-if="policyName === 'cssComplexSelectors' || policyName === 'cssComplexSelectorsByAttribute' || policyName === 'cssImports' || policyName === 'cssUniversalSelectors' || policyName === 'cssRedundantBodySelectors' || policyName === 'cssRedundantChildNodesSelectors'">
-                        <span ng-if="offender.bolded" ng-bind-html="offender.bolded"></span>
-                        <b ng-if="!offender.bolded">{{offender.css}}</b>
+                    <div ng-if="policyName === 'cssImports'">
+                        {{offender.css}}
                         <file-and-line-button file="offender.file" line="offender.line" column="offender.column"></file-and-line-button>
                     </div>
 
@@ -127,32 +121,6 @@
                         {{offender.rule}} (<b>x{{offender.occurrences}}</b>)
                     </div>
 
-                    <div ng-if="policyName === 'cssDuplicatedProperties'">
-                        Property <b>{{offender.property}}</b> duplicated in <b>{{offender.rule}} { }</b>
-                        <file-and-line-button file="offender.file" line="offender.line" column="offender.column"></file-and-line-button>
-                    </div>
-
-                    <div ng-if="policyName === 'cssEmptyRules'">
-                        <b>{{offender.css}} { }</b>
-                        <file-and-line-button file="offender.file" line="offender.line" column="offender.column"></file-and-line-button>
-                    </div>
-
-                    <div ng-if="policyName === 'cssExpressions'">
-                        {{offender.rule}} {{ '{' + offender.property}}: <b>expression(</b>{{offender.expression}}<b>)</b>}
-                        <file-and-line-button file="offender.file" line="offender.line" column="offender.column"></file-and-line-button>
-                    </div>
-
-                    <div ng-if="policyName === 'cssImportants'">
-                        {{offender.rule}} {{ '{' + offender.property}}: {{offender.value}} <b>!important</b>}
-                        <file-and-line-button file="offender.file" line="offender.line" column="offender.column"></file-and-line-button>
-                    </div>
-
-                    <div ng-if="policyName === 'cssOldIEFixes'">
-                        <span ng-if="offender.browser"><b>{{offender.browser}} fix:</b></span>
-                        <span ng-bind-html="offender.bolded"></span>
-                        <file-and-line-button file="offender.file" line="offender.line" column="offender.column"></file-and-line-button>
-                    </div>
-
                     <div ng-if="policyName === 'cssOldPropertyPrefixes'">
                         <b>{{offender.property}} {{offender.message}}</b>
                         <div ng-if="offender.rules.length" ng-click="offender.showMore = !offender.showMore" class="offenderButton">
@@ -195,7 +163,57 @@
             </div>
         </div>
 
-        <div ng-if="!rule.offendersObj.list" class="offendersHtml">
+        <div ng-repeat="(file, fileDetails) in rule.offendersObj.byFile track by $index">
+            <h3>
+                <ng-pluralize count="fileDetails.count" when="{'one': '1 offender', 'other': '{} offenders'}"></ng-pluralize>
+                in
+                <url-link ng-if="file !== 'Inline CSS'" url="file" max-length="80"></url-link>
+                <span ng-if="file === 'Inline CSS'">inline CSS</span>
+            </h3>
+
+            <div class="offendersTable">
+                <div ng-repeat="offender in fileDetails.offenders track by $index">
+                    <div ng-if="policyName === 'cssComplexSelectors' || policyName === 'cssComplexSelectorsByAttribute' || policyName === 'cssUniversalSelectors' || policyName === 'cssRedundantBodySelectors' || policyName === 'cssRedundantChildNodesSelectors'">
+                        <span ng-if="offender.bolded" ng-bind-html="offender.bolded"></span>
+                        <b ng-if="!offender.bolded">{{offender.css}}</b>
+                        <span ng-if="offender.line !== null && offender.column !== null"> @ {{offender.line}}:{{offender.column}}</span>
+                    </div>
+
+                    <div ng-if="policyName === 'cssMobileFirst'">
+                        <b>{{offender.query}}</b> for <ng-pluralize count="offender.rules" when="{'one':'1 rule','other':'{} rules'}"></ng-pluralize>
+                        <span ng-if="offender.line !== null && offender.column !== null"> @ {{offender.line}}:{{offender.column}}</span>
+                    </div>
+
+                    <div ng-if="policyName === 'cssDuplicatedProperties'">
+                        Property <b>{{offender.property}}</b> duplicated in <b>{{offender.rule}} { }</b>
+                        <span ng-if="offender.line !== null && offender.column !== null"> @ {{offender.line}}:{{offender.column}}</span>
+                    </div>
+
+                    <div ng-if="policyName === 'cssEmptyRules'">
+                        <b>{{offender.css}} { }</b>
+                        <span ng-if="offender.line !== null && offender.column !== null"> @ {{offender.line}}:{{offender.column}}</span>
+                    </div>
+
+                    <div ng-if="policyName === 'cssExpressions'">
+                        {{offender.rule}} {{ '{' + offender.property}}: <b>expression(</b>{{offender.expression}}<b>)</b>}
+                        <span ng-if="offender.line !== null && offender.column !== null"> @ {{offender.line}}:{{offender.column}}</span>
+                    </div>
+
+                    <div ng-if="policyName === 'cssImportants'">
+                        {{offender.rule}} {{ '{' + offender.property}}: {{offender.value}} <b>!important</b>}
+                        <span ng-if="offender.line !== null && offender.column !== null"> @ {{offender.line}}:{{offender.column}}</span>
+                    </div>
+
+                    <div ng-if="policyName === 'cssOldIEFixes'">
+                        <span ng-if="offender.browser"><b>{{offender.browser}} fix:</b></span>
+                        <span ng-bind-html="offender.bolded"></span>
+                        <span ng-if="offender.line !== null && offender.column !== null"> @ {{offender.line}}:{{offender.column}}</span>
+                    </div>
+                </div>
+            </div>
+        </div>
+
+        <div ng-if="!rule.offendersObj.list && !rule.offendersObj.byFile" class="offendersHtml">
             
             <div ng-if="policyName === 'DOMelementMaxDepth'">
                 <dom-tree tree="rule.offendersObj.tree"></dom-tree>

lib/metadata/policies.js

@@ -462,19 +462,18 @@ var policies = {
     "cssComplexSelectors": {
         "tool": "phantomas",
         "label": "Complex selectors",
-        "message": "<p>Complex selectors are CSS selectors with 4 or more expressions, like \"#header ul li .foo\".</p><p>They are adding more work for the browser, and this could be avoided by simplifying selectors.</p>",
+        "message": "<p>Complex selectors are CSS selectors with 4 or more expressions, like \"#header ul li .foo\".</p><p>They are adding more work for the browser, and this could be avoided by simplifying selectors. The <a href=\"http://getbem.com\" target=\"_blank\">B.E.M. methodology</a> is an useful way to simplify your CSS.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 600,
         "isAbnormalThreshold": 2000,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
-                    return splittedOffender;
-                })
-            };
+            var parsedOffenders = offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+                return splittedOffender;
+            });
+
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssComplexSelectorsByAttribute": {
@@ -486,16 +485,13 @@ var policies = {
         "isAbnormalThreshold": 150,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
-
-                    splittedOffender.bolded = splittedOffender.css.replace(/(\[[^ ]+[~\|\^\$\*]="[^"]+"\])/g, '<b>$1</b>');
+            var parsedOffenders = offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+                splittedOffender.bolded = splittedOffender.css.replace(/(\[[^ ]+[~\|\^\$\*]=["']?[^"'\]]+["']?\])/g, '<b>$1</b>');
+                return splittedOffender;
+            });
 
-                    return splittedOffender;
-                })
-            };
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssColors": {
@@ -590,10 +586,7 @@ var policies = {
         "isAbnormalThreshold": 1000,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders
-            };
+            return offendersHelpers.orderByFile(offenders);
         }
     },
     "cssImports": {
@@ -652,29 +645,28 @@ var policies = {
         "isAbnormalThreshold": 120,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
-
-                    var parts = /^([^{]+) {([^ ]+): (.+)}$/.exec(splittedOffender.css);
+            var parsedOffenders = offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
 
-                    if (!parts) {
-                        debug('cssDuplicatedProperties offenders transform function error with "%s"', offender);
-                        return {
-                            parseError: offender
-                        };
-                    }
+                var parts = /^([^{]+) {([^ ]+): (.+)}$/.exec(splittedOffender.css);
 
+                if (!parts) {
+                    debug('cssDuplicatedProperties offenders transform function error with "%s"', offender);
                     return {
-                        property: parts[2],
-                        rule: parts[1],
-                        file: splittedOffender.file,
-                        line: splittedOffender.line,
-                        column: splittedOffender.column
+                        parseError: offender
                     };
-                })
-            };
+                }
+
+                return {
+                    property: parts[2],
+                    rule: parts[1],
+                    file: splittedOffender.file,
+                    line: splittedOffender.line,
+                    column: splittedOffender.column
+                };
+            });
+
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssEmptyRules": {
@@ -686,14 +678,8 @@ var policies = {
         "isAbnormalThreshold": 100,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
-
-                    return splittedOffender;
-                })
-            };
+            var parsedOffenders = offenders.map(offendersHelpers.cssOffenderPattern);
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssExpressions": {
@@ -705,30 +691,29 @@ var policies = {
         "isAbnormalThreshold": 20,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
-
-                    var parts = /^(.*) {([^ ]+): expression\((.*)\)}$/.exec(splittedOffender.css);
+            var parsedOffenders = offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
 
-                    if (!parts) {
-                        debug('cssExpressions offenders transform function error with "%s"', offender);
-                        return {
-                            parseError: offender
-                        };
-                    }
+                var parts = /^(.*) {([^ ]+): expression\((.*)\)}$/.exec(splittedOffender.css);
 
+                if (!parts) {
+                    debug('cssExpressions offenders transform function error with "%s"', offender);
                     return {
-                        rule: parts[1],
-                        property: parts[2],
-                        expression: parts[3],
-                        file: splittedOffender.file,
-                        line: splittedOffender.line,
-                        column: splittedOffender.column
+                        parseError: offender
                     };
-                })
-            };
+                }
+
+                return {
+                    rule: parts[1],
+                    property: parts[2],
+                    expression: parts[3],
+                    file: splittedOffender.file,
+                    line: splittedOffender.line,
+                    column: splittedOffender.column
+                };
+            });
+
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssImportants": {
@@ -740,30 +725,29 @@ var policies = {
         "isAbnormalThreshold": 200,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
-
-                    var parts = /^(.*) {([^ ]+): (.*) ?\!important}$/.exec(splittedOffender.css);
+            var parsedOffenders = offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
 
-                    if (!parts) {
-                        debug('cssImportants offenders transform function error with "%s"', offender);
-                        return {
-                            parseError: offender
-                        };
-                    }
+                var parts = /^(.*) {([^ ]+): (.*) ?\!important}$/.exec(splittedOffender.css);
 
+                if (!parts) {
+                    debug('cssImportants offenders transform function error with "%s"', offender);
                     return {
-                        rule: parts[1],
-                        property: parts[2],
-                        value: parts[3],
-                        file: splittedOffender.file,
-                        line: splittedOffender.line,
-                        column: splittedOffender.column
+                        parseError: offender
                     };
-                })
-            };
+                }
+
+                return {
+                    rule: parts[1],
+                    property: parts[2],
+                    value: parts[3],
+                    file: splittedOffender.file,
+                    line: splittedOffender.line,
+                    column: splittedOffender.column
+                };
+            });
+
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssOldIEFixes": {
@@ -775,52 +759,51 @@ var policies = {
         "isAbnormalThreshold": 300,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+            var parsedOffenders = offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
 
-                    var parts = /^([^{]*)( {([^ ]+): (.*)})?$/.exec(splittedOffender.css);
+                var parts = /^([^{]*)( {([^ ]+): (.*)})?$/.exec(splittedOffender.css);
 
-                    if (!parts) {
-                        debug('cssOldIEFixes offenders transform function error with "%s"', offender);
-                        return {
-                            parseError: offender
-                        };
-                    }
+                if (!parts) {
+                    debug('cssOldIEFixes offenders transform function error with "%s"', offender);
+                    return {
+                        parseError: offender
+                    };
+                }
 
-                    var rule = parts[1];
-                    var property = parts[3];
-                    var value = parts[4];
-                    var browser = null;
-
-                    if (rule.indexOf('* html') === 0) {
-                        rule = rule.replace(/^\* html/, '<b>* html</b>');
-                        browser = 'IE6';
-                    } else if (rule.indexOf('html>body') === 0) {
-                        rule = rule.replace(/^html>body/, '<b>html>body</b>');
-                        browser = 'IE6';
-                    } else if (property.indexOf('*') === 0) {
-                        property = '<b>' + property + '</b>';
-                        browser = 'IE7';
-                    } else if (value.match(/\!ie$/)) {
-                        value = value.replace(/\!ie$/, '<b>!ie</b>');
-                        browser = 'IE7';
-                    } else if (property === '-ms-filter') {
-                        property = '<b>-ms-filter</b>';
-                        browser = 'IE9';
-                    } else if (value.indexOf('progid:DXImageTransform.Microsoft') >= 0) {
-                        value = value.replace(/progid:DXImageTransform\.Microsoft/, '<b>progid:DXImageTransform.Microsoft</b>');
-                        browser = 'IE9';
-                    }
+                var rule = parts[1];
+                var property = parts[3];
+                var value = parts[4];
+                var browser = null;
+
+                if (rule.indexOf('* html') === 0) {
+                    rule = rule.replace(/^\* html/, '<b>* html</b>');
+                    browser = 'IE6';
+                } else if (rule.indexOf('html>body') === 0) {
+                    rule = rule.replace(/^html>body/, '<b>html>body</b>');
+                    browser = 'IE6';
+                } else if (property.indexOf('*') === 0) {
+                    property = '<b>' + property + '</b>';
+                    browser = 'IE7';
+                } else if (value.match(/\!ie$/)) {
+                    value = value.replace(/\!ie$/, '<b>!ie</b>');
+                    browser = 'IE7';
+                } else if (property === '-ms-filter') {
+                    property = '<b>-ms-filter</b>';
+                    browser = 'IE9';
+                } else if (value.indexOf('progid:DXImageTransform.Microsoft') >= 0) {
+                    value = value.replace(/progid:DXImageTransform\.Microsoft/, '<b>progid:DXImageTransform.Microsoft</b>');
+                    browser = 'IE9';
+                }
 
-                    var propertyAndValue = (property && value) ? ' {' + property + ': ' + value + '}' : '';
-                    splittedOffender.bolded = rule + propertyAndValue;
-                    splittedOffender.browser = browser;
+                var propertyAndValue = (property && value) ? ' {' + property + ': ' + value + '}' : '';
+                splittedOffender.bolded = rule + propertyAndValue;
+                splittedOffender.browser = browser;
 
-                    return splittedOffender;
-                })
-            };
+                return splittedOffender;
+            });
+
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssOldPropertyPrefixes": {
@@ -885,13 +868,9 @@ var policies = {
         "isAbnormalThreshold": 150,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
-                    return splittedOffender;
-                })
-            };
+            var parsedOffenders = offenders.map(offendersHelpers.cssOffenderPattern);
+
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssRedundantBodySelectors": {
@@ -903,16 +882,13 @@ var policies = {
         "isAbnormalThreshold": 200,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
-
-                    splittedOffender.bolded = splittedOffender.css.replace(/body/, '<b>body</b>');
+            var parsedOffenders = offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+                splittedOffender.bolded = splittedOffender.css.replace(/body/, '<b>body</b>');
+                return splittedOffender;
+            });
 
-                    return splittedOffender;
-                })
-            };
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "cssRedundantChildNodesSelectors": {
@@ -924,29 +900,28 @@ var policies = {
         "isAbnormalThreshold": 200,
         "hasOffenders": true,
         "offendersTransformFn": function(offenders) {
-            return {
-                count: offenders.length,
-                list: offenders.map(function(offender) {
-                    var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
+            var parsedOffenders = offenders.map(function(offender) {
+                var splittedOffender = offendersHelpers.cssOffenderPattern(offender);
 
-                    var rule = splittedOffender.css || '';
-                    var redundanters = [
-                        ['ul', 'li'],
-                        ['ol', 'li'],
-                        ['select', 'option'],
-                        ['table', 'tr'],
-                        ['table', 'th'],
-                    ];
+                var rule = splittedOffender.css || '';
+                var redundanters = [
+                    ['ul', 'li'],
+                    ['ol', 'li'],
+                    ['select', 'option'],
+                    ['table', 'tr'],
+                    ['table', 'th'],
+                ];
+
+                redundanters.forEach(function(couple) {
+                    rule = rule.replace(new RegExp('(^| |>)' + couple[0] + '([^ >]*)?([ >]| > )' + couple[1] + '([^\\w-]|$)', 'g'), '$1<b>' + couple[0] + '</b>$2$3<b>' + couple[1] + '</b>$4');
+                });
 
-                    redundanters.forEach(function(couple) {
-                        rule = rule.replace(new RegExp('(^| |>)' + couple[0] + '([^ >]*)?([ >]| > )' + couple[1] + '([^\\w-]|$)', 'g'), '$1<b>' + couple[0] + '</b>$2$3<b>' + couple[1] + '</b>$4');
-                    });
+                splittedOffender.bolded = rule;
 
-                    splittedOffender.bolded = rule;
+                return splittedOffender;
+            });
 
-                    return splittedOffender;
-                })
-            };
+            return offendersHelpers.orderByFile(parsedOffenders);
         }
     },
     "totalWeight": {

lib/offendersHelpers.js

@@ -185,6 +185,27 @@ var OffendersHelpers = function() {
         }
     };
 
+    this.orderByFile = function(offenders) {
+        var byFile = {};
+
+        offenders.forEach(function(offender) {
+            var file = offender.file || 'Inline CSS';
+            delete offender.file;
+
+            if (!byFile[file]) {
+                byFile[file] = {
+                    count: 0,
+                    offenders: []
+                };
+            }
+
+            byFile[file].count ++;
+            byFile[file].offenders.push(offender);
+        });
+
+        return {byFile: byFile};
+    };
+
 };
 
 module.exports = new OffendersHelpers();