From 87a70f335984b1664723895bdc5bdc6e6af83715 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 1 Mar 2019 22:28:19 +0100 Subject: [PATCH 1/2] Convert `let` to `const` if possible in `src/display/display_utils.js` Finally, `var` usage is removed. --- src/display/display_utils.js | 54 +++++++++++++++++------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/display/display_utils.js b/src/display/display_utils.js index faaeedd9d..d779aa7a5 100644 --- a/src/display/display_utils.js +++ b/src/display/display_utils.js @@ -12,6 +12,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +/* eslint no-var: error */ import { assert, CMapCompressionType, removeNullCharacters, stringToBytes, @@ -24,10 +25,10 @@ const SVG_NS = 'http://www.w3.org/2000/svg'; class DOMCanvasFactory { create(width, height) { if (width <= 0 || height <= 0) { - throw new Error('invalid canvas size'); + throw new Error('Invalid canvas size'); } - let canvas = document.createElement('canvas'); - let context = canvas.getContext('2d'); + const canvas = document.createElement('canvas'); + const context = canvas.getContext('2d'); canvas.width = width; canvas.height = height; return { @@ -38,10 +39,10 @@ class DOMCanvasFactory { reset(canvasAndContext, width, height) { if (!canvasAndContext.canvas) { - throw new Error('canvas is not specified'); + throw new Error('Canvas is not specified'); } if (width <= 0 || height <= 0) { - throw new Error('invalid canvas size'); + throw new Error('Invalid canvas size'); } canvasAndContext.canvas.width = width; canvasAndContext.canvas.height = height; @@ -49,7 +50,7 @@ class DOMCanvasFactory { destroy(canvasAndContext) { if (!canvasAndContext.canvas) { - throw new Error('canvas is not specified'); + throw new Error('Canvas is not specified'); } // Zeroing the width and height cause Firefox to release graphics // resources immediately, which can greatly reduce memory consumption. @@ -137,7 +138,7 @@ class DOMSVGFactory { create(width, height) { assert(width > 0 && height > 0, 'Invalid SVG dimensions'); - let svg = document.createElementNS(SVG_NS, 'svg:svg'); + const svg = document.createElementNS(SVG_NS, 'svg:svg'); svg.setAttribute('version', '1.1'); svg.setAttribute('width', width + 'px'); svg.setAttribute('height', height + 'px'); @@ -194,8 +195,8 @@ class PageViewport { // creating transform to convert pdf coordinate system to the normal // canvas like coordinates taking in account scale and rotation - let centerX = (viewBox[2] + viewBox[0]) / 2; - let centerY = (viewBox[3] + viewBox[1]) / 2; + const centerX = (viewBox[2] + viewBox[0]) / 2; + const centerY = (viewBox[3] + viewBox[1]) / 2; let rotateA, rotateB, rotateC, rotateD; rotation = rotation % 360; rotation = rotation < 0 ? rotation + 360 : rotation; @@ -287,9 +288,9 @@ class PageViewport { * @see {@link convertToViewportPoint} */ convertToViewportRectangle(rect) { - let tl = Util.applyTransform([rect[0], rect[1]], this.transform); - let br = Util.applyTransform([rect[2], rect[3]], this.transform); - return [tl[0], tl[1], br[0], br[1]]; + const topLeft = Util.applyTransform([rect[0], rect[1]], this.transform); + const bottomRight = Util.applyTransform([rect[2], rect[3]], this.transform); + return [topLeft[0], topLeft[1], bottomRight[0], bottomRight[1]]; } /** @@ -306,7 +307,7 @@ class PageViewport { } } -var RenderingCancelledException = (function RenderingCancelledException() { +const RenderingCancelledException = (function RenderingCancelledException() { function RenderingCancelledException(msg, type) { this.message = msg; this.type = type; @@ -332,7 +333,7 @@ const LinkTargetStringMap = [ '_self', '_blank', '_parent', - '_top' + '_top', ]; /** @@ -355,7 +356,7 @@ function addLinkAttributes(link, { url, target, rel, } = {}) { if (url) { const LinkTargetValues = Object.values(LinkTarget); - let targetIndex = + const targetIndex = LinkTargetValues.includes(target) ? target : LinkTarget.NONE; link.target = LinkTargetStringMap[targetIndex]; @@ -365,11 +366,10 @@ function addLinkAttributes(link, { url, target, rel, } = {}) { // Gets the file name from a given URL. function getFilenameFromUrl(url) { - var anchor = url.indexOf('#'); - var query = url.indexOf('?'); - var end = Math.min( - anchor > 0 ? anchor : url.length, - query > 0 ? query : url.length); + const anchor = url.indexOf('#'); + const query = url.indexOf('?'); + const end = Math.min(anchor > 0 ? anchor : url.length, + query > 0 ? query : url.length); return url.substring(url.lastIndexOf('/', end) + 1, end); } @@ -407,19 +407,17 @@ class StatTimer { } toString() { - let times = this.times; // Find the longest name for padding purposes. let out = '', longest = 0; - for (let i = 0, ii = times.length; i < ii; ++i) { - let name = times[i]['name']; + for (const time of this.times) { + const name = time.name; if (name.length > longest) { longest = name.length; } } - for (let i = 0, ii = times.length; i < ii; ++i) { - let span = times[i]; - let duration = span.end - span.start; - out += `${span['name'].padEnd(longest)} ${duration}ms\n`; + for (const time of this.times) { + const duration = time.end - time.start; + out += `${time.name.padEnd(longest)} ${duration}ms\n`; } return out; } @@ -466,7 +464,7 @@ function isValidFetchUrl(url, baseUrl) { function loadScript(src) { return new Promise((resolve, reject) => { - let script = document.createElement('script'); + const script = document.createElement('script'); script.src = src; script.onload = resolve; From b244622f7e9744e06a8680ad885fe55b4c284c9e Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 1 Mar 2019 23:08:34 +0100 Subject: [PATCH 2/2] Improve unit test coverage for `src/display/display_utils.js` The `DOMCanvasFactory` class is now fully covered. Moreover, missing cases for the `getFilenameFromUrl` function have been included. Finally, `var` usage has been removed. --- test/unit/display_utils_spec.js | 132 ++++++++++++++++++++++++++++---- 1 file changed, 117 insertions(+), 15 deletions(-) diff --git a/test/unit/display_utils_spec.js b/test/unit/display_utils_spec.js index 67f339fa7..5dbbb4093 100644 --- a/test/unit/display_utils_spec.js +++ b/test/unit/display_utils_spec.js @@ -12,22 +12,120 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +/* eslint no-var: error */ import { - DOMSVGFactory, getFilenameFromUrl, isValidFetchUrl + DOMCanvasFactory, DOMSVGFactory, getFilenameFromUrl, isValidFetchUrl } from '../../src/display/display_utils'; import isNodeJS from '../../src/shared/is_node'; describe('display_utils', function() { + describe('DOMCanvasFactory', function() { + let canvasFactory; + + beforeAll(function(done) { + canvasFactory = new DOMCanvasFactory(); + done(); + }); + + afterAll(function() { + canvasFactory = null; + }); + + it('`create` should throw an error if the dimensions are invalid', + function() { + // Invalid width. + expect(function() { + return canvasFactory.create(-1, 1); + }).toThrow(new Error('Invalid canvas size')); + + // Invalid height. + expect(function() { + return canvasFactory.create(1, -1); + }).toThrow(new Error('Invalid canvas size')); + }); + + it('`create` should return a canvas if the dimensions are valid', + function() { + if (isNodeJS()) { + pending('Document is not supported in Node.js.'); + } + + const { canvas, context, } = canvasFactory.create(20, 40); + expect(canvas instanceof HTMLCanvasElement).toBe(true); + expect(context instanceof CanvasRenderingContext2D).toBe(true); + expect(canvas.width).toBe(20); + expect(canvas.height).toBe(40); + }); + + it('`reset` should throw an error if no canvas is provided', function() { + const canvasAndContext = { canvas: null, context: null, }; + + expect(function() { + return canvasFactory.reset(canvasAndContext, 20, 40); + }).toThrow(new Error('Canvas is not specified')); + }); + + it('`reset` should throw an error if the dimensions are invalid', + function() { + const canvasAndContext = { canvas: 'foo', context: 'bar', }; + + // Invalid width. + expect(function() { + return canvasFactory.reset(canvasAndContext, -1, 1); + }).toThrow(new Error('Invalid canvas size')); + + // Invalid height. + expect(function() { + return canvasFactory.reset(canvasAndContext, 1, -1); + }).toThrow(new Error('Invalid canvas size')); + }); + + it('`reset` should alter the canvas/context if the dimensions are valid', + function() { + if (isNodeJS()) { + pending('Document is not supported in Node.js.'); + } + + const canvasAndContext = canvasFactory.create(20, 40); + canvasFactory.reset(canvasAndContext, 60, 80); + + const { canvas, context, } = canvasAndContext; + expect(canvas instanceof HTMLCanvasElement).toBe(true); + expect(context instanceof CanvasRenderingContext2D).toBe(true); + expect(canvas.width).toBe(60); + expect(canvas.height).toBe(80); + }); + + it('`destroy` should throw an error if no canvas is provided', function() { + expect(function() { + return canvasFactory.destroy({}); + }).toThrow(new Error('Canvas is not specified')); + }); + + it('`destroy` should clear the canvas/context', function() { + if (isNodeJS()) { + pending('Document is not supported in Node.js.'); + } + + const canvasAndContext = canvasFactory.create(20, 40); + canvasFactory.destroy(canvasAndContext); + + const { canvas, context, } = canvasAndContext; + expect(canvas).toBe(null); + expect(context).toBe(null); + }); + }); + describe('DOMSVGFactory', function() { let svgFactory; - beforeAll(function (done) { + beforeAll(function(done) { svgFactory = new DOMSVGFactory(); done(); }); - afterAll(function () { + afterAll(function() { svgFactory = null; }); @@ -50,8 +148,7 @@ describe('display_utils', function() { pending('Document is not supported in Node.js.'); } - let svg = svgFactory.create(20, 40); - + const svg = svgFactory.create(20, 40); expect(svg instanceof SVGSVGElement).toBe(true); expect(svg.getAttribute('version')).toBe('1.1'); expect(svg.getAttribute('width')).toBe('20px'); @@ -73,25 +170,30 @@ describe('display_utils', function() { pending('Document is not supported in Node.js.'); } - let svg = svgFactory.createElement('svg:rect'); - + const svg = svgFactory.createElement('svg:rect'); expect(svg instanceof SVGRectElement).toBe(true); }); }); describe('getFilenameFromUrl', function() { it('should get the filename from an absolute URL', function() { - var url = 'http://server.org/filename.pdf'; - var result = getFilenameFromUrl(url); - var expected = 'filename.pdf'; - expect(result).toEqual(expected); + const url = 'https://server.org/filename.pdf'; + expect(getFilenameFromUrl(url)).toEqual('filename.pdf'); }); it('should get the filename from a relative URL', function() { - var url = '../../filename.pdf'; - var result = getFilenameFromUrl(url); - var expected = 'filename.pdf'; - expect(result).toEqual(expected); + const url = '../../filename.pdf'; + expect(getFilenameFromUrl(url)).toEqual('filename.pdf'); + }); + + it('should get the filename from a URL with an anchor', function() { + const url = 'https://server.org/filename.pdf#foo'; + expect(getFilenameFromUrl(url)).toEqual('filename.pdf'); + }); + + it('should get the filename from a URL with query parameters', function() { + const url = 'https://server.org/filename.pdf?foo=bar'; + expect(getFilenameFromUrl(url)).toEqual('filename.pdf'); }); });