From 4ef1a129fa21862f7af036a427a6ca31a57fea1e Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 16 Nov 2021 12:36:22 +0100 Subject: [PATCH 1/2] Replace the remaining `Node.removeChild()` instances with `Element.remove()` Using `Element.remove()` is a slightly more compact way of removing an element, since you no longer need to explicitly find/use its parent element. Furthermore, the patch also replaces a couple of loops that're used to delete all elements under a node with simply overwriting the contents directly (a pattern already used throughout the viewer). See also: - https://developer.mozilla.org/en-US/docs/Web/API/Node/removeChild - https://developer.mozilla.org/en-US/docs/Web/API/Element/remove --- extensions/chromium/contentscript.js | 2 +- src/display/font_loader.js | 2 +- test/driver.js | 2 +- test/resources/reftest-analyzer.js | 4 +--- web/debugger.js | 7 ++----- web/pdf_page_view.js | 6 +++--- 6 files changed, 9 insertions(+), 14 deletions(-) diff --git a/extensions/chromium/contentscript.js b/extensions/chromium/contentscript.js index a29963e78..674c274ab 100644 --- a/extensions/chromium/contentscript.js +++ b/extensions/chromium/contentscript.js @@ -128,7 +128,7 @@ function updateEmbedElement(elem) { var parentNode = elem.parentNode; var nextSibling = elem.nextSibling; if (parentNode) { - parentNode.removeChild(elem); + elem.remove(); } elem.type = "text/html"; elem.src = getEmbeddedViewerURL(elem.src); diff --git a/src/display/font_loader.js b/src/display/font_loader.js index 2536a943b..0b6aedc45 100644 --- a/src/display/font_loader.js +++ b/src/display/font_loader.js @@ -350,7 +350,7 @@ if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) { this._document.body.appendChild(div); isFontReady(loadTestFontId, () => { - this._document.body.removeChild(div); + div.remove(); request.complete(); }); /** Hack end */ diff --git a/test/driver.js b/test/driver.js index 0c8550486..83d8283e7 100644 --- a/test/driver.js +++ b/test/driver.js @@ -565,7 +565,7 @@ var Driver = (function DriverClosure() { } const body = document.body; while (body.lastChild !== this.end) { - body.removeChild(body.lastChild); + body.lastChild.remove(); } const destroyedPromises = []; diff --git a/test/resources/reftest-analyzer.js b/test/resources/reftest-analyzer.js index b0968dd09..2a192f4a0 100644 --- a/test/resources/reftest-analyzer.js +++ b/test/resources/reftest-analyzer.js @@ -245,9 +245,7 @@ window.onload = function () { // const cell = ID("itemlist"); const table = document.getElementById("itemtable"); - while (table.childNodes.length > 0) { - table.removeChild(table.childNodes[table.childNodes.length - 1]); - } + table.textContent = ""; // Remove any table contents from the DOM. const tbody = document.createElement("tbody"); table.appendChild(tbody); diff --git a/web/debugger.js b/web/debugger.js index 216e01a22..9686807e4 100644 --- a/web/debugger.js +++ b/web/debugger.js @@ -462,9 +462,7 @@ const Stepper = (function StepperClosure() { var Stats = (function Stats() { let stats = []; function clear(node) { - while (node.hasChildNodes()) { - node.removeChild(node.lastChild); - } + node.textContent = ""; // Remove any `node` contents from the DOM. } function getStatIndex(pageNumber) { for (let i = 0, ii = stats.length; i < ii; ++i) { @@ -490,8 +488,7 @@ var Stats = (function Stats() { } const statsIndex = getStatIndex(pageNumber); if (statsIndex !== false) { - const b = stats[statsIndex]; - this.panel.removeChild(b.div); + stats[statsIndex].div.remove(); stats.splice(statsIndex, 1); } const wrapper = document.createElement("div"); diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index ca87ab4d8..c01ad0266 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -257,7 +257,7 @@ class PDFPageView { case xfaLayerNode: continue; } - div.removeChild(node); + node.remove(); } div.removeAttribute("data-loaded"); @@ -534,7 +534,7 @@ class PDFPageView { this.renderingState = RenderingStates.FINISHED; if (this.loadingIconDiv) { - div.removeChild(this.loadingIconDiv); + this.loadingIconDiv.remove(); delete this.loadingIconDiv; } return Promise.reject(new Error("pdfPage is not loaded")); @@ -617,7 +617,7 @@ class PDFPageView { this.renderingState = RenderingStates.FINISHED; if (this.loadingIconDiv) { - div.removeChild(this.loadingIconDiv); + this.loadingIconDiv.remove(); delete this.loadingIconDiv; } this._resetZoomLayer(/* removeFromDOM = */ true); From 4e2c2fafc9fe28a2ddeed300fb0fad7a173a6cce Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 16 Nov 2021 15:51:33 +0100 Subject: [PATCH 2/2] Enable the `unicorn/prefer-dom-node-remove` ESLint plugin rule Please see https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-dom-node-remove.md --- .eslintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintrc b/.eslintrc index ae3143d3d..22ade7ddb 100644 --- a/.eslintrc +++ b/.eslintrc @@ -50,6 +50,7 @@ "unicorn/no-instanceof-array": "error", "unicorn/no-useless-spread": "error", "unicorn/prefer-date-now": "error", + "unicorn/prefer-dom-node-remove": "error", "unicorn/prefer-string-starts-ends-with": "error", // Possible errors