From f75e99f703389b09af8573ccab55884f853b16e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Fri, 23 Oct 2015 17:31:00 +0200 Subject: [PATCH 1/8] Fix bug where scrollExecutionTree is empty --- lib/tools/jsExecutionTransformer.js | 36 ++++++++++++++++------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/tools/jsExecutionTransformer.js b/lib/tools/jsExecutionTransformer.js index a215341..2a6214e 100644 --- a/lib/tools/jsExecutionTransformer.js +++ b/lib/tools/jsExecutionTransformer.js @@ -121,24 +121,28 @@ var jsExecutionTransformer = function() { debug('JS execution transformation complete'); - debug('Starting scroll execution transformation'); - offenders.DOMaccessesOnScroll = JSON.parse(data.toolsResults.phantomas.offenders.scrollExecutionTree[0]); - if (offenders.DOMaccessesOnScroll.children) { - offenders.DOMaccessesOnScroll.children.forEach(function(node) { - - // Mark a event flag - if (['documentScroll', 'windowScroll', 'window.onscroll'].indexOf(node.data.type) >= 0) { - node.windowPerformance = true; - } + if (data.toolsResults.phantomas.offenders.scrollExecutionTree) { + debug('Starting scroll execution transformation'); + offenders.DOMaccessesOnScroll = JSON.parse(data.toolsResults.phantomas.offenders.scrollExecutionTree[0]); + if (offenders.DOMaccessesOnScroll.children) { + offenders.DOMaccessesOnScroll.children.forEach(function(node) { + + // Mark a event flag + if (['documentScroll', 'windowScroll', 'window.onscroll'].indexOf(node.data.type) >= 0) { + node.windowPerformance = true; + } - // Transform domPaths into objects - changeListOfDomPaths(node); - - // Count the number of DOM accesses, by counting the tree leafs - metrics.DOMaccessesOnScroll += countTreeLeafs(node); - }); + // Transform domPaths into objects + changeListOfDomPaths(node); + + // Count the number of DOM accesses, by counting the tree leafs + metrics.DOMaccessesOnScroll += countTreeLeafs(node); + }); + } + debug('Scroll execution transformation complete'); + } else { + debug('Could not parse scrollExecutionTree'); } - debug('Scroll execution transformation complete'); } catch(err) { throw err; From 835cb2f90fd251479dd67253a006c1122240dc5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Wed, 28 Oct 2015 12:04:00 +0100 Subject: [PATCH 2/8] Improve Old prefixes offenders readability --- front/src/css/rule.css | 5 ++++ front/src/less/rule.less | 5 ++++ front/src/views/rule.html | 16 +++++++--- lib/metadata/policies.js | 63 +++++++++++++++++++++++++-------------- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/front/src/css/rule.css b/front/src/css/rule.css index b9b3014..bef4497 100644 --- a/front/src/css/rule.css +++ b/front/src/css/rule.css @@ -101,6 +101,7 @@ margin: 0.2em 0; border-radius: 0.4em; z-index: 1; + cursor: pointer; } .offendersTable .offenderButton.opens, .value .offenderButton.opens { @@ -157,6 +158,10 @@ display: block; background: #ffe0cc; } +.offendersTable .smallerOffenders, +.value .smallerOffenders { + font-size: 0.9em; +} .offendersHtml { display: inline-block; } diff --git a/front/src/less/rule.less b/front/src/less/rule.less index c3da1e9..a4310ed 100644 --- a/front/src/less/rule.less +++ b/front/src/less/rule.less @@ -112,6 +112,7 @@ margin: 0.2em 0; border-radius: 0.4em; z-index: 1; + cursor: pointer; &.opens { padding-right: 0.75em; @@ -166,6 +167,10 @@ } } } + + .smallerOffenders { + font-size: 0.9em; + } } .offendersHtml { diff --git a/front/src/views/rule.html b/front/src/views/rule.html index 001b0f7..19dc91e 100644 --- a/front/src/views/rule.html +++ b/front/src/views/rule.html @@ -143,10 +143,18 @@
- {{offender.rule}} {{{offender.property}}: {{offender.value + '}' }} -
- {{offender.message}} - + {{offender.property}} {{offender.message}} +
+ show + hide + +
+
+
+ {{cssRule.rule}} {{'{' + offender.property}}: {{cssRule.value + '}' }} + +
+
diff --git a/lib/metadata/policies.js b/lib/metadata/policies.js index ae59880..17d3000 100644 --- a/lib/metadata/policies.js +++ b/lib/metadata/policies.js @@ -752,36 +752,53 @@ var policies = { "cssOldPropertyPrefixes": { "tool": "phantomas", "label": "Old prefixes", - "message": "

Many property prefixes such as -moz- or -webkit- are not needed anymore, or by very few people. You can remove them or replace them with the non-prefixed version. This will help reducing your stylesheets weight.

", + "message": "

Many property prefixes such as -moz- or -webkit- are not needed anymore, or by very few people. Sometimes, they have never even existed. You can remove them or replace them with the non-prefixed version. This will help reducing your stylesheets weight.

The prefixes database comes from Can I Use.

", "isOkThreshold": 0, "isBadThreshold": 75, "isAbnormalThreshold": 300, "hasOffenders": true, "offendersTransformFn": function(offenders) { + var properties = {}; + offenders.forEach(function(offender) { + var splittedOffender = offendersHelpers.cssOffenderPattern(offender); + + var parts = /^([^{]*)(?: ?{ ?([^ ]+): (.*) ?}) \/\/ (.*)$/.exec(splittedOffender.css); + + if (!parts) { + debug('cssOldPropertyPrefixes offenders transform function error with "%s"', offender); + return { + parseError: offender + }; + } + + var propertyName = parts[2]; + + if (!properties[propertyName]) { + properties[propertyName] = { + property: propertyName, + message: parts[4], + rules: [] + }; + } + + properties[propertyName].rules.push({ + rule: parts[1], + value: parts[3], + file: splittedOffender.file, + line: splittedOffender.line, + column: splittedOffender.column + }); + }); + + // Object to array + var list = []; + for (var propertyName in properties) { + list.push(properties[propertyName]); + } + return { count: offenders.length, - list: offenders.map(function(offender) { - var splittedOffender = offendersHelpers.cssOffenderPattern(offender); - - var parts = /^([^{]*)(?: ?{ ?([^ ]+): (.*) ?}) \/\/ (.*)$/.exec(splittedOffender.css); - - if (!parts) { - debug('cssOldPropertyPrefixes offenders transform function error with "%s"', offender); - return { - parseError: offender - }; - } - - return { - rule: parts[1], - property: parts[2], - value: parts[3], - message: parts[4], - file: splittedOffender.file, - line: splittedOffender.line, - column: splittedOffender.column - }; - }) + list: list }; } }, From b67efe0cac5a9a66d9505038d52d2ef2753fb698 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Wed, 28 Oct 2015 20:05:43 +0100 Subject: [PATCH 3/8] Update dependencies --- package.json | 54 ++++++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/package.json b/package.json index 1e15436..883deef 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "yellowlabtools", - "version": "1.8.0", + "version": "1.8.1", "description": "Online tool to audit a webpage for performance and front-end quality issues", "license": "GPL-2.0", "author": { @@ -15,51 +15,55 @@ "bin": { "yellowlabtools": "./bin/cli.js" }, + "engines": { + "node": ">= 0.12.0" + }, "main": "./lib/index.js", "dependencies": { - "angular": "1.4.5", - "angular-animate": "1.4.5", - "angular-chart.js": "0.8.4", + "angular": "1.4.7", + "angular-animate": "1.4.7", + "angular-chart.js": "0.8.5", "angular-local-storage": "0.2.2", - "angular-resource": "1.4.5", - "angular-route": "1.4.5", - "angular-sanitize": "1.4.5", - "async": "1.4.2", - "body-parser": "1.14.0", + "angular-resource": "1.4.7", + "angular-route": "1.4.7", + "angular-sanitize": "1.4.7", + "async": "1.5.0", + "body-parser": "1.14.1", "chart.js": "1.0.2", - "clean-css": "3.4.3", + "clean-css": "3.4.6", "color-diff": "0.1.7", - "compression": "1.5.2", + "compression": "1.6.0", "cors": "2.7.1", "debug": "2.2.0", "express": "4.13.3", - "imagemin": "3.2.0", + "imagemin": "3.2.2", "imagemin-jpegoptim": "4.0.0", "jstoxml": "0.2.3", - "lwip": "0.0.7", - "meow": "3.3.0", - "minimize": "1.7.1", + "lwip": "0.0.8", + "meow": "3.4.2", + "minimize": "1.7.4", "parse-color": "1.0.0", - "phantomas": "1.12.0", - "ps-node": "0.0.4", + "phantomas": "1.13.0", + "ps-node": "0.0.5", "q": "1.4.1", - "request": "2.62.0", + "request": "2.65.0", "rimraf": "2.4.3", "temporary": "0.0.8", - "uglify-js": "2.4.24" + "try-thread-sleep": "1.0.0", + "uglify-js": "2.5.0" }, "devDependencies": { - "chai": "~3.2.0", + "chai": "~3.4.0", "grunt": "~0.4.5", "grunt-blanket": "~0.0.8", "grunt-contrib-clean": "~0.6.0", "grunt-contrib-concat": "~0.5.1", - "grunt-contrib-copy": "~0.8.1", + "grunt-contrib-copy": "~0.8.2", "grunt-contrib-cssmin": "~0.14.0", - "grunt-contrib-htmlmin": "~0.4.0", + "grunt-contrib-htmlmin": "~0.6.0", "grunt-contrib-jshint": "~0.11.3", "grunt-contrib-less": "~1.0.1", - "grunt-contrib-uglify": "~0.9.2", + "grunt-contrib-uglify": "~0.10.0", "grunt-env": "~0.4.4", "grunt-express": "~1.4.1", "grunt-filerev": "~2.3.1", @@ -69,9 +73,9 @@ "grunt-replace": "~0.11.0", "grunt-usemin": "~3.1.1", "grunt-webfont": "~0.5.4", - "matchdep": "~0.3.0", + "matchdep": "~1.0.0", "mocha": "~2.3.2", - "sinon": "~1.16.1", + "sinon": "~1.17.2", "sinon-chai": "~2.8.0" }, "scripts": { From d8730cc67beca0c3a95f46827e465532e2f6c41c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Wed, 28 Oct 2015 20:07:02 +0100 Subject: [PATCH 4/8] Fix tests after phantomas v1.13 --- lib/tools/phantomas/phantomasWrapper.js | 19 +++++++------- test/core/phantomasWrapperTest.js | 33 +++++++++++++++---------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/lib/tools/phantomas/phantomasWrapper.js b/lib/tools/phantomas/phantomasWrapper.js index 65d53d7..b83ca33 100644 --- a/lib/tools/phantomas/phantomasWrapper.js +++ b/lib/tools/phantomas/phantomasWrapper.js @@ -115,6 +115,7 @@ var PhantomasWrapper = function() { async.retry(triesNumber, function(cb) { var process = phantomas(task.url, options, function(err, json, results) { + var errorCode = err ? parseInt(err.message, 10) : null; if (isKilled) { debug('Process was killed, too late Phantomas, sorry...'); @@ -122,28 +123,28 @@ var PhantomasWrapper = function() { } - debug('Returning from Phantomas with error %s', err); + debug('Returning from Phantomas with error %s', errorCode); // Adding some YellowLabTools errors here if (json && json.metrics && (!json.metrics.javascriptExecutionTree || !json.offenders.javascriptExecutionTree)) { - err = 1001; + errorCode = 1001; } - if (!err && (!json || !json.metrics)) { - err = 1002; + if (!errorCode && (!json || !json.metrics)) { + errorCode = 1002; } // Don't cancel test if it is a timeout and we've got some results - if (err === 252 && json) { + if (errorCode === 252 && json) { debug('Timeout after ' + options.timeout + ' seconds. But it\'s not a problem, the test is valid.'); - err = null; + errorCode = null; } - if (err) { - debug('Attempt failed. Error code ' + err); + if (errorCode) { + debug('Attempt failed. Error code ' + errorCode); } - cb(err, json); + cb(errorCode, json); }); phantomasPid = process.pid; diff --git a/test/core/phantomasWrapperTest.js b/test/core/phantomasWrapperTest.js index ef5cef8..b6d6372 100644 --- a/test/core/phantomasWrapperTest.js +++ b/test/core/phantomasWrapperTest.js @@ -51,11 +51,14 @@ describe('phantomasWrapper', function() { done('Error: unwanted success'); }).fail(function(err) { + try { + should.exist(err); + err.should.equal(254); - should.exist(err); - err.should.equal(254); - - done(); + done(); + } catch(error) { + done(error); + } }); }); @@ -72,16 +75,20 @@ describe('phantomasWrapper', function() { } }).then(function(data) { /*jshint -W030 */ - - data.should.be.an('object'); - data.should.have.a.property('generator'); - data.generator.should.contain('phantomas'); - data.should.have.a.property('url').that.equals(url); - data.should.have.a.property('metrics').that.is.an('object').not.empty; - data.should.have.a.property('offenders').that.is.an('object').not.empty; - data.offenders.should.have.a.property('javascriptExecutionTree').that.is.a('array').not.empty; - done(); + try { + data.should.be.an('object'); + data.should.have.a.property('generator'); + data.generator.should.contain('phantomas'); + data.should.have.a.property('url').that.equals(url); + data.should.have.a.property('metrics').that.is.an('object').not.empty; + data.should.have.a.property('offenders').that.is.an('object').not.empty; + data.offenders.should.have.a.property('javascriptExecutionTree').that.is.a('array').not.empty; + + done(); + } catch(error) { + done(error); + } }).fail(function(err) { done(err); }); From 6f7f05b4564275bf7e60919b941111c16979e3e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Wed, 28 Oct 2015 22:09:26 +0100 Subject: [PATCH 5/8] Small fix for old prefixes offenders --- front/src/views/rule.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/front/src/views/rule.html b/front/src/views/rule.html index 19dc91e..d888850 100644 --- a/front/src/views/rule.html +++ b/front/src/views/rule.html @@ -144,12 +144,12 @@
{{offender.property}} {{offender.message}} -
- show - hide +
+ show + hide
-
+
{{cssRule.rule}} {{'{' + offender.property}}: {{cssRule.value + '}' }} From c54cf5a2bad7b9330c6fef28e8f4741fb7099fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Wed, 28 Oct 2015 22:55:52 +0100 Subject: [PATCH 6/8] Relaunch run on webkit1 if webkit2 fails --- lib/tools/phantomas/phantomasWrapper.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/tools/phantomas/phantomasWrapper.js b/lib/tools/phantomas/phantomasWrapper.js index b83ca33..e739a90 100644 --- a/lib/tools/phantomas/phantomasWrapper.js +++ b/lib/tools/phantomas/phantomasWrapper.js @@ -112,8 +112,17 @@ var PhantomasWrapper = function() { // It's time to launch the test!!! var triesNumber = 2; + var currentTry = 0; async.retry(triesNumber, function(cb) { + + currentTry ++; + // Fix for https://github.com/gmetais/YellowLabTools/issues/114 + if (currentTry === 2 && options.engine === 'webkit2') { + debug('Launching a second try with the old webkit v1 engine'); + options.engine = 'webkit'; + } + var process = phantomas(task.url, options, function(err, json, results) { var errorCode = err ? parseInt(err.message, 10) : null; From 0eb359542b93ef709739e3f2c88c6f01a3ca3fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Mon, 2 Nov 2015 18:56:33 +0100 Subject: [PATCH 7/8] Temporary fix for relative lazyloadable images src --- .../lazyLoadableYLT/lazyLoadableYLT.js | 78 +++++++++++++++++++ lib/tools/phantomas/phantomasWrapper.js | 1 + 2 files changed, 79 insertions(+) create mode 100644 lib/tools/phantomas/custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js diff --git a/lib/tools/phantomas/custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js b/lib/tools/phantomas/custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js new file mode 100644 index 0000000..0547d98 --- /dev/null +++ b/lib/tools/phantomas/custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js @@ -0,0 +1,78 @@ +/** + * Analyzes images and detects which one can be lazy-loaded (are below the fold) + * + * @see https://github.com/macbre/phantomas/issues/494 + */ +/* global document: true, window: true */ +'use strict'; + +exports.version = '1.0.a'; + +exports.module = function(phantomas) { + phantomas.setMetric('lazyLoadableImagesBelowTheFold'); // @desc number of images displayed below the fold that can be lazy-loaded + + phantomas.on('report', function() { + phantomas.log('lazyLoadableImages: analyzing which images can be lazy-loaded...'); + + phantomas.evaluate(function() { + (function(phantomas) { + phantomas.spyEnabled(false, 'analyzing which images can be lazy-loaded'); + + var images = document.body.getElementsByTagName('img'), + i, + len = images.length, + offset, + path, + processedImages = {}, + src, + viewportHeight = window.innerHeight; + + phantomas.log('lazyLoadableImages: %d image(s) found, assuming %dpx offset to be the fold', len, viewportHeight); + + for (i = 0; i < len; i++) { + // @see https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect + offset = images[i].getBoundingClientRect().top; + src = images[i].src; + + // ignore base64-encoded images + if (src === '' || /^data:/.test(src)) { + continue; + } + + path = phantomas.getDOMPath(images[i]); + + // get the most top position for a given image (deduplicate by src) + if (typeof processedImages[src] === 'undefined') { + processedImages[src] = { + offset: offset, + path: path + }; + } + + // maybe there's the same image loaded above the fold? + if (offset < processedImages[src].offset) { + processedImages[src] = { + offset: offset, + path: path + }; + } + } + + phantomas.log('lazyLoadableImages: checking %d unique image(s)', Object.keys(processedImages).length); + + Object.keys(processedImages).forEach(function(src) { + var img = processedImages[src]; + + if (img.offset > viewportHeight) { + phantomas.log('lazyLoadableImages: <%s> image (%s) is below the fold (at %dpx)', src, img.path, img.offset); + + phantomas.incrMetric('lazyLoadableImagesBelowTheFold'); + phantomas.addOffender('lazyLoadableImagesBelowTheFold', src); + } + }); + + phantomas.spyEnabled(true); + })(window.__phantomas); + }); + }); +}; \ No newline at end of file diff --git a/lib/tools/phantomas/phantomasWrapper.js b/lib/tools/phantomas/phantomasWrapper.js index e739a90..9660ae2 100644 --- a/lib/tools/phantomas/phantomasWrapper.js +++ b/lib/tools/phantomas/phantomasWrapper.js @@ -46,6 +46,7 @@ var PhantomasWrapper = function() { 'javaScriptBottlenecks', // needs to be launched after custom module scopeYLT 'jQuery', // overridden 'jserrors', // overridden + 'lazyLoadableImages', //overriden 'pageSource', // not needed 'windowPerformance' // overriden ].join(','), From e78e794240b8328123d9545e7280d0ad8269c8b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20M=C3=A9tais?= Date: Mon, 2 Nov 2015 19:07:35 +0100 Subject: [PATCH 8/8] Fix jsHint --- .../custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/tools/phantomas/custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js b/lib/tools/phantomas/custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js index 0547d98..c3cfc45 100644 --- a/lib/tools/phantomas/custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js +++ b/lib/tools/phantomas/custom_modules/modules/lazyLoadableYLT/lazyLoadableYLT.js @@ -4,11 +4,12 @@ * @see https://github.com/macbre/phantomas/issues/494 */ /* global document: true, window: true */ -'use strict'; exports.version = '1.0.a'; exports.module = function(phantomas) { + 'use strict'; + phantomas.setMetric('lazyLoadableImagesBelowTheFold'); // @desc number of images displayed below the fold that can be lazy-loaded phantomas.on('report', function() {