From 9b0ed6f821219ee07ca8684b7a99b6dd3b4d03c3 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 21 Jun 2018 18:15:04 +0200 Subject: [PATCH] Remove all pages from the DOM at once, rather than using a loop, in `PDFViewer._regroupSpreads` There's no good reason to iterate through an arbitrary number of DOM elements this way, since a document could contain thousands of pages, when everything can be easily removed at once; compare with e.g. `BaseViewer._resetView` and `PDFThumbnailViewer._resetView`. Furthermore given that it's a `PDFViewer` instance, the `this.viewer` property can be accessed directly. Besides, `_setDocumentViewerElement` only exists as a helper method for `setDocument` in the base class and none of this code applies for `PDFSinglePageViewer` instances either. --- web/pdf_viewer.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 2a6c46ca8..99757087d 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -88,13 +88,13 @@ class PDFViewer extends BaseViewer { } _regroupSpreads() { - const container = this._setDocumentViewerElement, pages = this._pages; - while (container.firstChild) { - container.firstChild.remove(); - } + const viewer = this.viewer, pages = this._pages; + // Temporarily remove all the pages from the DOM. + viewer.textContent = ''; + if (this.spreadMode === SpreadMode.NONE) { for (let i = 0, iMax = pages.length; i < iMax; ++i) { - container.appendChild(pages[i].div); + viewer.appendChild(pages[i].div); } } else { const parity = this.spreadMode - 1; @@ -103,10 +103,10 @@ class PDFViewer extends BaseViewer { if (spread === null) { spread = document.createElement('div'); spread.className = 'spread'; - container.appendChild(spread); + viewer.appendChild(spread); } else if (i % 2 === parity) { spread = spread.cloneNode(false); - container.appendChild(spread); + viewer.appendChild(spread); } spread.appendChild(pages[i].div); }