From e830ac9dd8cbff90f5520d5490999ef78cc52cf4 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Tue, 8 Mar 2016 14:32:00 +0000 Subject: [PATCH 1/3] Fix eslint issues identified in m-c --- .eslintrc | 194 ++++++++++++++++++++++++++++++++++++++ JSDOMParser.js | 14 +-- Readability.js | 24 +++-- test/generate-testcase.js | 2 +- 4 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 .eslintrc diff --git a/.eslintrc b/.eslintrc new file mode 100644 index 0000000..135b51e --- /dev/null +++ b/.eslintrc @@ -0,0 +1,194 @@ +{ + "rules": { + // Braces only needed for multi-line arrow function blocks + // "arrow-body-style": [2, "as-needed"], + + // Require spacing around => + // "arrow-spacing": 2, + + // Always require spacing around a single line block + // "block-spacing": 1, + + // No newline before open brace for a block + // "brace-style": 2, + + // No space before always a space after a comma + // "comma-spacing": [2, {"before": false, "after": true}], + + // Commas at the end of the line not the start + // "comma-style": 2, + + // Don't require spaces around computed properties + // "computed-property-spacing": [2, "never"], + + // Functions must always return something or nothing + "consistent-return": 2, + + // Require braces around blocks that start a new line + // Note that this rule is likely to be overridden on a per-directory basis + // very frequently. + // "curly": [2, "multi-line"], + + // Always require a trailing EOL + "eol-last": 2, + + // Require function* name() + // "generator-star-spacing": [2, {"before": false, "after": true}], + + // Two space indent + // "indent": [2, 2, { "SwitchCase": 1 }], + + // Space after colon not before in property declarations + // "key-spacing": [2, { "beforeColon": false, "afterColon": true, "mode": "minimum" }], + + // Unix linebreaks + "linebreak-style": [2, "unix"], + + // Always require parenthesis for new calls + // "new-parens": 2, + + // Use [] instead of Array() + // "no-array-constructor": 2, + + // No duplicate arguments in function declarations + "no-dupe-args": 2, + + // No duplicate keys in object declarations + "no-dupe-keys": 2, + + // No duplicate cases in switch statements + "no-duplicate-case": 2, + + // No labels + "no-labels": 2, + + // If an if block ends with a return no need for an else block + // "no-else-return": 2, + + // No empty statements + // "no-empty": 2, + + // No empty character classes in regex + "no-empty-character-class": 2, + + // Disallow empty destructuring + "no-empty-pattern": 2, + + // No assiging to exception variable + // "no-ex-assign": 2, + + // No using !! where casting to boolean is already happening + // "no-extra-boolean-cast": 2, + + // No double semicolon + "no-extra-semi": 2, + + // No overwriting defined functions + "no-func-assign": 2, + + // No invalid regular expresions + "no-invalid-regexp": 2, + + // No odd whitespace characters + "no-irregular-whitespace": 2, + + // No single if block inside an else block + // "no-lonely-if": 2, + + // No mixing spaces and tabs in indent + "no-mixed-spaces-and-tabs": [2, "smart-tabs"], + + // No unnecessary spacing + // "no-multi-spaces": [2, { exceptions: { "AssignmentExpression": true, "VariableDeclarator": true, "ArrayExpression": true, "ObjectExpression": true } }], + + // No reassigning native JS objects + "no-native-reassign": 2, + + // No (!foo in bar) + "no-negated-in-lhs": 2, + + // Nested ternary statements are confusing + "no-nested-ternary": 2, + + // Use {} instead of new Object() + // "no-new-object": 2, + + // No Math() or JSON() + "no-obj-calls": 2, + + // No octal literals + "no-octal": 2, + + // No redeclaring variables + "no-redeclare": 2, + + // No unnecessary comparisons + "no-self-compare": 2, + + // No declaring variables from an outer scope + // "no-shadow": 2, + + // No declaring variables that hide things like arguments + // "no-shadow-restricted-names": 2, + + // No spaces between function name and parentheses + // "no-spaced-func": 2, + + // No trailing whitespace + "no-trailing-spaces": 2, + + // No using undeclared variables + // "no-undef": 2, + + // Error on newline where a semicolon is needed + "no-unexpected-multiline": 2, + + // No unreachable statements + // "no-unreachable": 2, + + // No expressions where a statement is expected + // "no-unused-expressions": 2, + + // No declaring variables that are never used + // "no-unused-vars": [2, {"vars": "all", "args": "none"}], + + // No using variables before defined + // "no-use-before-define": [2, "nofunc"], + + // No using with + "no-with": 2, + + // Always require semicolon at end of statement + // "semi": [2, "always"], + + // Require space after keywords + // "space-after-keywords": 2, + + // Require space before blocks + // "space-before-blocks": 2, + + // Never use spaces before function parentheses + // "space-before-function-paren": [2, { "anonymous": "always", "named": "never" }], + + // Require spaces before finally, catch, etc. + // "space-before-keywords": [2, "always"], + + // No space padding in parentheses + // "space-in-parens": [2, "never"], + + // Require spaces around operators + // "space-infix-ops": 2, + + // Require spaces after return, throw and case + // "space-return-throw-case": 2, + + // ++ and -- should not need spacing + // "space-unary-ops": [2, { "words": true, "nonwords": false }], + + // No comparisons to NaN + "use-isnan": 2, + + // Only check typeof against valid results + "valid-typeof": 2, + }, +} diff --git a/JSDOMParser.js b/JSDOMParser.js index 11ef588..58b7584 100644 --- a/JSDOMParser.js +++ b/JSDOMParser.js @@ -1,3 +1,4 @@ +/*eslint-env es6:false*/ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ @@ -691,12 +692,13 @@ set innerHTML(html) { var parser = new JSDOMParser(); var node = parser.parse(html); - for (var i = this.childNodes.length; --i >= 0;) { + var i; + for (i = this.childNodes.length; --i >= 0;) { this.childNodes[i].parentNode = null; } this.childNodes = node.childNodes; this.children = node.children; - for (var i = this.childNodes.length; --i >= 0;) { + for (i = this.childNodes.length; --i >= 0;) { this.childNodes[i].parentNode = this; } }, @@ -1081,16 +1083,16 @@ // Read any text as Text node if (c !== "<") { --this.currentChar; - var node = new Text(); + var textNode = new Text(); var n = this.html.indexOf("<", this.currentChar); if (n === -1) { - node.innerHTML = this.html.substring(this.currentChar, this.html.length); + textNode.innerHTML = this.html.substring(this.currentChar, this.html.length); this.currentChar = this.html.length; } else { - node.innerHTML = this.html.substring(this.currentChar, n); + textNode.innerHTML = this.html.substring(this.currentChar, n); this.currentChar = n; } - return node; + return textNode; } c = this.peekNext(); diff --git a/Readability.js b/Readability.js index f617fda..fed6dad 100644 --- a/Readability.js +++ b/Readability.js @@ -1,3 +1,4 @@ +/*eslint-env es6:false*/ /* * Copyright (c) 2010 Arc90 Inc * @@ -65,8 +66,11 @@ var Readability = function(uri, doc, options) { return rv + '("' + e.textContent + '")'; } var classDesc = e.className && ("." + e.className.replace(/ /g, ".")); - var elDesc = e.id ? "(#" + e.id + classDesc + ")" : - (classDesc ? "(" + classDesc + ")" : ""); + var elDesc = ""; + if (e.id) + elDesc = "(#" + e.id + classDesc + ")"; + else if (classDesc) + elDesc = "(" + classDesc + ")"; return rv + elDesc; } this.log = function () { @@ -368,9 +372,9 @@ Readability.prototype = { // (which will be replaced with a

later). while ((next = this._nextElement(next)) && (next.tagName == "BR")) { replaced = true; - var sibling = next.nextSibling; + var brSibling = next.nextSibling; next.parentNode.removeChild(next); - next = sibling; + next = brSibling; } // If we removed a
chain, replace the remaining
with a

. Add @@ -741,7 +745,12 @@ Readability.prototype = { // - parent: 1 (no division) // - grandparent: 2 // - great grandparent+: ancestor level * 3 - var scoreDivider = level === 0 ? 1 : level === 1 ? 2 : level * 3; + if (level === 0) + var scoreDivider = 1; + else if (level === 1) + scoreDivider = 2; + else + scoreDivider = level * 3; ancestor.readability.contentScore += contentScore / scoreDivider; }); }); @@ -854,7 +863,8 @@ Readability.prototype = { if (nodeLength > 80 && linkDensity < 0.25) { append = true; - } else if (nodeLength < 80 && linkDensity === 0 && nodeContent.search(/\.( |$)/) !== -1) { + } else if (nodeLength < 80 && nodeLength > 0 && linkDensity === 0 && + nodeContent.search(/\.( |$)/) !== -1) { append = true; } } @@ -1139,7 +1149,7 @@ Readability.prototype = { _getLinkDensity: function(element) { var textLength = this._getInnerText(element).length; if (textLength === 0) - return; + return 0; var linkLength = 0; diff --git a/test/generate-testcase.js b/test/generate-testcase.js index ae9ba4d..c599ac2 100644 --- a/test/generate-testcase.js +++ b/test/generate-testcase.js @@ -17,7 +17,7 @@ var FFX_UA = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100 if (process.argv.length < 3) { console.error("Need at least a destination slug and potentially a URL (if the slug doesn't have source)."); process.exit(0); - return; + throw "Abort"; } var slug = process.argv[2]; From a4d1e9ca12c25c9855264be0cfbd1c9eb479f9c4 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Thu, 10 Mar 2016 14:40:12 +0000 Subject: [PATCH 2/3] Fix oversight in comment removal code exposed by better/newer jsdom implementation --- test/test-readability.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/test-readability.js b/test/test-readability.js index 1225dbe..c7d4f8f 100644 --- a/test/test-readability.js +++ b/test/test-readability.js @@ -77,6 +77,9 @@ function runTestsWithItems(label, domGenerationFn, uri, source, expectedContent, if (n.nodeType == 3) { return "#text(" + htmlTransform(n.textContent) + ")"; } + if (n.nodeType != 1) { + return "some other node type: " + n.nodeType + " with data " + n.data; + } var rv = n.localName; if (n.id) { rv += "#" + n.id; @@ -141,13 +144,14 @@ function runTestsWithItems(label, domGenerationFn, uri, source, expectedContent, } function removeCommentNodesRecursively(node) { - [].forEach.call(node.childNodes, function(child) { + for (var i = node.childNodes.length - 1; i >= 0; i--) { + var child = node.childNodes[i]; if (child.nodeType === child.COMMENT_NODE) { node.removeChild(child); } else if (child.nodeType === child.ELEMENT_NODE) { removeCommentNodesRecursively(child); } - }); + } } describe("Readability API", function() { From 7579ac4ea8705a4e5139d3c7caa599aae14cb710 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Thu, 10 Mar 2016 13:44:06 +0000 Subject: [PATCH 3/3] Enable linting through travis --- .travis.yml | 5 ++++- package.json | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 41735d7..11d7ff7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,7 @@ language: node_js sudo: false node_js: - - '0.10' + - '5.3' +script: + - npm run lint + - npm run test diff --git a/package.json b/package.json index a6fb6ea..e7ceab5 100644 --- a/package.json +++ b/package.json @@ -4,6 +4,7 @@ "description": "A standalone version of the readability library used for Firefox Reader View.", "main": "Readability.js", "scripts": { + "lint": "eslint .", "test": "mocha test/test-*.js", "generate-testcase": "node test/generate-testcase.js", "perf": "matcha benchmarks/benchmarks.js", @@ -21,8 +22,9 @@ "homepage": "https://github.com/mozilla/readability", "devDependencies": { "chai": "^2.1.*", + "eslint": ">1.10", "js-beautify": "^1.5.5", - "jsdom": "^3.1.2", + "jsdom": "^7.0", "matcha": "^0.6.0", "mocha": "^2.2.*" }