From 14513cfc37dbff305b5a793d8f95ef9587e3e669 Mon Sep 17 00:00:00 2001 From: Daniel Weck Date: Wed, 18 Sep 2024 20:08:20 +0100 Subject: [PATCH] fix: null vs. undefined vs. falsy checks, report description was incorrectly verified (was using title), error first discovered in log provided at #402 --- packages/ace-axe-runner-electron/bin/ace.js | 3 ++- packages/ace-core/src/checker/checker-chromium.js | 6 +++--- packages/ace-core/src/checker/checker-epub.js | 4 ++-- packages/ace-core/src/checker/checker.js | 4 ++-- packages/ace-core/src/core/ace.js | 4 ++-- packages/ace-http/src/index.js | 6 +++--- packages/ace-report/src/report-builders.js | 10 +++++----- packages/ace-report/src/report.js | 2 +- packages/epub-utils/src/epub.js | 4 ++-- scripts/translate-scan.js | 2 +- tests/data/report/report.html | 6 +++--- 11 files changed, 26 insertions(+), 25 deletions(-) diff --git a/packages/ace-axe-runner-electron/bin/ace.js b/packages/ace-axe-runner-electron/bin/ace.js index 2dc69ef8..9deb30d4 100644 --- a/packages/ace-axe-runner-electron/bin/ace.js +++ b/packages/ace-axe-runner-electron/bin/ace.js @@ -18,9 +18,10 @@ var args = [].concat(path.resolve(__dirname, "../lib/cli.js"), process.argv.slic var child = proc.spawn(electron, args, { stdio: 'inherit', windowsHide: false }) child.on('close', function (code, signal) { - if (code === null) { + if (code === null || code === undefined) { console.error(electron, 'exited with signal', signal) process.exit(1) + return; } process.exit(code) }) diff --git a/packages/ace-core/src/checker/checker-chromium.js b/packages/ace-core/src/checker/checker-chromium.js index d1104742..224709dc 100644 --- a/packages/ace-core/src/checker/checker-chromium.js +++ b/packages/ace-core/src/checker/checker-chromium.js @@ -124,10 +124,10 @@ async function checkSingle(spineItem, epub, lang, axeRunner) { Object.getOwnPropertyNames(results.data).forEach((key) => { if (!Array.isArray(results.data[key])) return; results.data[key].forEach((item) => { - if (item.src !== undefined) { + if (item.src) { if (Array.isArray(item.src)) { item.src = item.src.map((srcItem) => { - if (srcItem.src !== undefined) { + if (srcItem.src) { if (LOG_DEBUG_URLS) { console.log("----- ITEMs SRC 1"); console.log(srcItem.src); @@ -161,7 +161,7 @@ async function checkSingle(spineItem, epub, lang, axeRunner) { console.log(item.src); } } - if (item.cfi !== undefined) { + if (item.cfi) { if (LOG_DEBUG_URLS) { console.log("----- CFI 1"); console.log(spineItem.relpath); diff --git a/packages/ace-core/src/checker/checker-epub.js b/packages/ace-core/src/checker/checker-epub.js index 81e7c71e..4604a060 100644 --- a/packages/ace-core/src/checker/checker-epub.js +++ b/packages/ace-core/src/checker/checker-epub.js @@ -15,7 +15,7 @@ const KB_BASE = 'http://kb.daisy.org/publishing/'; function asString(arrayOrString) { if (Array.isArray(arrayOrString) && arrayOrString.length > 0) { return asString(arrayOrString[0]); - } else if (arrayOrString !== undefined) { + } else if (arrayOrString) { return arrayOrString.toString().trim(); } return ''; @@ -71,7 +71,7 @@ function checkMetadata(assertions, epub) { for (const name in a11yMetadata.A11Y_META) { const meta = a11yMetadata.A11Y_META[name]; var values = epub.metadata[name]; - if (values === undefined) { + if (!values) { // Report missing metadata if it is required or recommended if (meta.required) assertions.withAssertions(newMetadataAssertion(name)); if (meta.recommended) assertions.withAssertions(newMetadataAssertion(name, 'moderate')); diff --git a/packages/ace-core/src/checker/checker.js b/packages/ace-core/src/checker/checker.js index 3b81ec31..fe5ab111 100644 --- a/packages/ace-core/src/checker/checker.js +++ b/packages/ace-core/src/checker/checker.js @@ -26,7 +26,7 @@ function consolidate(results, report) { } return undefined; })) - .filter(e => e !== undefined); + .filter(e => e); report.addHeadings(headings); // Aggregated array of the HTML outlines const htmlOutlines = [] @@ -36,7 +36,7 @@ function consolidate(results, report) { } return undefined; })) - .filter(e => e !== undefined); + .filter(e => e); report.addHTMLOutlines(htmlOutlines); return report; } diff --git a/packages/ace-core/src/core/ace.js b/packages/ace-core/src/core/ace.js index 8b4efaab..e1b1a2d3 100644 --- a/packages/ace-core/src/core/ace.js +++ b/packages/ace-core/src/core/ace.js @@ -46,7 +46,7 @@ module.exports = function ace(epubPath, options, axeRunner) { if (!fs.existsSync(options.tmpdir)) { fs.ensureDirSync(options.tmpdir); } - } else if (options.tmpdir === undefined) { + } else if (!options.tmpdir) { options.tmpdir = tmp.dirSync({ unsafeCleanup: true }).name; } if (typeof options.outdir === 'string') { @@ -72,7 +72,7 @@ module.exports = function ace(epubPath, options, axeRunner) { .then(report => checker.check(epub, report, options.lang, axeRunner)) // Process the Results .then((report) => { - if (options.outdir === undefined) { + if (!options.outdir) { report.cleanData(); process.stdout.write(`${JSON.stringify(report.json, null, ' ')}\n`); return report; diff --git a/packages/ace-http/src/index.js b/packages/ace-http/src/index.js index 9762ffbf..edbc39dd 100644 --- a/packages/ace-http/src/index.js +++ b/packages/ace-http/src/index.js @@ -115,7 +115,7 @@ function initRoutes() { // return the job information function getJob(req, res, next) { var jobdata = joblist.find(jobdata => jobdata.internal.id === req.params.jobid); - if (jobdata == undefined || jobdata == null) { + if (!jobdata) { res.sendStatus(404); // not found } else { @@ -135,7 +135,7 @@ function getJobs(req, res, next) { // return the job information function postJob(req, res, next) { - if (req.file == undefined) { + if (!req.file) { res.sendStatus(400); // bad request } else { @@ -164,7 +164,7 @@ function postJob(req, res, next) { // return the report as either a zipfile or a json object, depending on what was specifically requested function getReport(req, res) { var jobdata = joblist.find(jobdata => jobdata.internal.id === req.params.jobid); - if (jobdata == undefined || jobdata == null) { + if (!jobdata) { res.sendStatus(404); // not found } else { diff --git a/packages/ace-report/src/report-builders.js b/packages/ace-report/src/report-builders.js index 08aabbc5..a6c9b370 100644 --- a/packages/ace-report/src/report-builders.js +++ b/packages/ace-report/src/report-builders.js @@ -35,8 +35,8 @@ function withTestSubject(obj, url, title = '', version='', identifier = '', meta const testSubject = { url }; if (title.length > 0) testSubject['dct:title'] = title; if (identifier.length > 0) testSubject['dct:identifier'] = identifier; - if (metadata !== undefined && metadata != null) testSubject.metadata = metadata; - if (links !== undefined && links != null) testSubject.links = links; + if (metadata) testSubject.metadata = metadata; + if (links) testSubject.links = links; if (version && typeof version === "string" && version.length > 0) testSubject.epubVersion = version; obj['earl:testSubject'] = testSubject; return obj; @@ -91,8 +91,8 @@ class ReportBuilder { this._json = { '@type': 'earl:report', '@context': 'http://daisy.github.io/ace/ace-report-1.0.jsonld', - 'dct:title': (title == null) ? '' : title.toString(), - 'dct:description': (title == null) ? '' : description.toString(), + 'dct:title': !title ? '' : title.toString(), + 'dct:description': !description ? '' : description.toString(), 'dct:date': new Date().toLocaleString(), 'earl:assertedBy': { '@type': 'earl:software', @@ -171,7 +171,7 @@ class ReportBuilder { } withTestSubject(url, title, version, identifier, metadata, links) { var url_ = url; - if (this.outdir !== undefined && this.outdir != "" && reportConfig["use-relative-paths"]) { + if (this.outdir && reportConfig["use-relative-paths"]) { url_ = path.relative(this.outdir, url); } withTestSubject(this._json, url_, title, version, identifier, metadata, links); diff --git a/packages/ace-report/src/report.js b/packages/ace-report/src/report.js index dd60bfc1..c4ed3996 100644 --- a/packages/ace-report/src/report.js +++ b/packages/ace-report/src/report.js @@ -111,7 +111,7 @@ module.exports = class Report { } copyData(outdir) { winston.info("Copying data"); - if (this.json.data === null) return Promise.resolve(); + if (!this.json.data) return Promise.resolve(); return fs.pathExists(outdir) .then((exists) => { if (!exists) return fs.ensureDirSync(outdir); diff --git a/packages/epub-utils/src/epub.js b/packages/epub-utils/src/epub.js index dfb3a223..726db435 100644 --- a/packages/epub-utils/src/epub.js +++ b/packages/epub-utils/src/epub.js @@ -71,7 +71,7 @@ async function unzip(unzipDir, path, useLegacyZipLib) { } async function retryUnzip(unzipDir, epub, error) { - if (error.message === undefined) throw error; + if (!error.message) throw error; winston.info('Trying to repair the archive and unzip again...'); try { // Detect 'invalid comment length' errors @@ -128,7 +128,7 @@ class EPUB { } async extract(unzipDir) { - if (this.basedir !== undefined) { + if (this.basedir) { return this; } else if (this.expanded) { winston.verbose('EPUB is already unpacked'); diff --git a/scripts/translate-scan.js b/scripts/translate-scan.js index f1695cf7..5aa6eb9c 100644 --- a/scripts/translate-scan.js +++ b/scripts/translate-scan.js @@ -8,7 +8,7 @@ const srcFolderPath = args[0]; const jsonFilePath = args[1]; function isNullOrUndefined(val) { - return val === undefined && val === null; + return val === undefined || val === null; } function sortObject(obj) { diff --git a/tests/data/report/report.html b/tests/data/report/report.html index 68c129b9..9445a042 100644 --- a/tests/data/report/report.html +++ b/tests/data/report/report.html @@ -138,17 +138,17 @@ tr.append(tdImage); - var tdAlt = (image.alt === undefined) ? + var tdAlt = !image.alt ? $('N/A') : $('' + image.alt + ''); tr.append(tdAlt); - var tdDescribedby = (image.describedby === undefined) ? + var tdDescribedby = !image.describedby ? $('N/A') : $('' + image.describedby + ''); tr.append(tdDescribedby); - var tdFigcaption = (image.figcaption === undefined) ? + var tdFigcaption = !image.figcaption ? $('N/A') : $('' + image.figcaption + ''); tr.append(tdFigcaption);