From 32f1d0de76d6566e62cc98f4351ed383337ed24b Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 5 Apr 2020 10:24:02 +0200 Subject: [PATCH 1/4] Move the initialization of "page labels" out of `PDFViewerApplication.load` Over time, with more and more API-functionality added, the `PDFViewerApplication.load` method has become quite large and complex. In an attempt to improve the current situation somewhat, this patch moves the fetching and initialization of "page labels" out into its own (private) helper method instead. --- web/app.js | 76 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/web/app.js b/web/app.js index 155462fa3..aa8abfed3 100644 --- a/web/app.js +++ b/web/app.js @@ -1185,39 +1185,6 @@ const PDFViewerApplication = { }); }); - pdfDocument.getPageLabels().then(labels => { - if (!labels || AppOptions.get("disablePageLabels")) { - return; - } - const numLabels = labels.length; - if (numLabels !== this.pagesCount) { - console.error( - "The number of Page Labels does not match " + - "the number of pages in the document." - ); - return; - } - let i = 0; - // Ignore page labels that correspond to standard page numbering. - while (i < numLabels && labels[i] === (i + 1).toString()) { - i++; - } - if (i === numLabels) { - return; - } - - pdfViewer.setPageLabels(labels); - pdfThumbnailViewer.setPageLabels(labels); - - // Changing toolbar page display to use labels and we need to set - // the label of the current page. - this.toolbar.setPagesCount(pdfDocument.numPages, true); - this.toolbar.setPageNumber( - pdfViewer.currentPageNumber, - pdfViewer.currentPageLabel - ); - }); - pagesPromise.then(async () => { const [openAction, javaScript] = await Promise.all([ openActionPromise, @@ -1269,6 +1236,8 @@ const PDFViewerApplication = { }); }); + this._initializePageLabels(pdfDocument); + pdfDocument .getMetadata() .then(({ info, metadata, contentDispositionFilename }) => { @@ -1408,6 +1377,47 @@ const PDFViewerApplication = { }); }, + /** + * @private + */ + async _initializePageLabels(pdfDocument) { + const labels = await pdfDocument.getPageLabels(); + + if (pdfDocument !== this.pdfDocument) { + return; // The document was closed while the page labels resolved. + } + if (!labels || AppOptions.get("disablePageLabels")) { + return; + } + const numLabels = labels.length; + if (numLabels !== this.pagesCount) { + console.error( + "The number of Page Labels does not match the number of pages in the document." + ); + return; + } + let i = 0; + // Ignore page labels that correspond to standard page numbering. + while (i < numLabels && labels[i] === (i + 1).toString()) { + i++; + } + if (i === numLabels) { + return; + } + const { pdfViewer, pdfThumbnailViewer, toolbar } = this; + + pdfViewer.setPageLabels(labels); + pdfThumbnailViewer.setPageLabels(labels); + + // Changing toolbar page display to use labels and we need to set + // the label of the current page. + toolbar.setPagesCount(numLabels, true); + toolbar.setPageNumber( + pdfViewer.currentPageNumber, + pdfViewer.currentPageLabel + ); + }, + /** * @private */ From d07be1a89bed0b78dcd18f5134f6601caac0b9fb Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 5 Apr 2020 10:26:55 +0200 Subject: [PATCH 2/4] Move the initialization of "metadata" out of `PDFViewerApplication.load` Over time, with more and more API-functionality added, the `PDFViewerApplication.load` method has become quite large and complex. In an attempt to improve the current situation somewhat, this patch moves the fetching and initialization of "metadata" out into its own (private) helper method instead. --- web/app.js | 273 ++++++++++++++++++++++++++++------------------------- 1 file changed, 142 insertions(+), 131 deletions(-) diff --git a/web/app.js b/web/app.js index aa8abfed3..f0e64db3c 100644 --- a/web/app.js +++ b/web/app.js @@ -1237,144 +1237,155 @@ const PDFViewerApplication = { }); this._initializePageLabels(pdfDocument); + this._initializeMetadata(pdfDocument); + }, - pdfDocument - .getMetadata() - .then(({ info, metadata, contentDispositionFilename }) => { - this.documentInfo = info; - this.metadata = metadata; - this.contentDispositionFilename = contentDispositionFilename; + /** + * @private + */ + async _initializeMetadata(pdfDocument) { + const { + info, + metadata, + contentDispositionFilename, + } = await pdfDocument.getMetadata(); - // Provides some basic debug information - console.log( - "PDF " + - pdfDocument.fingerprint + - " [" + - info.PDFFormatVersion + - " " + - (info.Producer || "-").trim() + - " / " + - (info.Creator || "-").trim() + - "]" + - " (PDF.js: " + - (version || "-") + - (AppOptions.get("enableWebGL") ? " [WebGL]" : "") + - ")" - ); + if (pdfDocument !== this.pdfDocument) { + return; // The document was closed while the metadata resolved. + } + this.documentInfo = info; + this.metadata = metadata; + this.contentDispositionFilename = contentDispositionFilename; - let pdfTitle; + // Provides some basic debug information + console.log( + "PDF " + + pdfDocument.fingerprint + + " [" + + info.PDFFormatVersion + + " " + + (info.Producer || "-").trim() + + " / " + + (info.Creator || "-").trim() + + "]" + + " (PDF.js: " + + (version || "-") + + (AppOptions.get("enableWebGL") ? " [WebGL]" : "") + + ")" + ); - const infoTitle = info && info["Title"]; - if (infoTitle) { - pdfTitle = infoTitle; - } - const metadataTitle = metadata && metadata.get("dc:title"); - if (metadataTitle) { - // Ghostscript can produce invalid 'dc:title' Metadata entries: - // - The title may be "Untitled" (fixes bug 1031612). - // - The title may contain incorrectly encoded characters, which thus - // looks broken, hence we ignore the Metadata entry when it - // contains characters from the Specials Unicode block - // (fixes bug 1605526). - if ( - metadataTitle !== "Untitled" && - !/[\uFFF0-\uFFFF]/g.test(metadataTitle) - ) { - pdfTitle = metadataTitle; + let pdfTitle; + + const infoTitle = info && info["Title"]; + if (infoTitle) { + pdfTitle = infoTitle; + } + const metadataTitle = metadata && metadata.get("dc:title"); + if (metadataTitle) { + // Ghostscript can produce invalid 'dc:title' Metadata entries: + // - The title may be "Untitled" (fixes bug 1031612). + // - The title may contain incorrectly encoded characters, which thus + // looks broken, hence we ignore the Metadata entry when it + // contains characters from the Specials Unicode block + // (fixes bug 1605526). + if ( + metadataTitle !== "Untitled" && + !/[\uFFF0-\uFFFF]/g.test(metadataTitle) + ) { + pdfTitle = metadataTitle; + } + } + + if (pdfTitle) { + this.setTitle( + `${pdfTitle} - ${contentDispositionFilename || document.title}` + ); + } else if (contentDispositionFilename) { + this.setTitle(contentDispositionFilename); + } + + if (info.IsAcroFormPresent) { + console.warn("Warning: AcroForm/XFA is not supported"); + this.fallback(UNSUPPORTED_FEATURES.forms); + } + + if ( + typeof PDFJSDev === "undefined" || + PDFJSDev.test("MOZCENTRAL || GENERIC") + ) { + // Telemetry labels must be C++ variable friendly. + let versionId = "other"; + // Keep these in sync with mozilla central's Histograms.json. + const KNOWN_VERSIONS = [ + "1.0", + "1.1", + "1.2", + "1.3", + "1.4", + "1.5", + "1.6", + "1.7", + "1.8", + "1.9", + "2.0", + "2.1", + "2.2", + "2.3", + ]; + if (KNOWN_VERSIONS.includes(info.PDFFormatVersion)) { + versionId = `v${info.PDFFormatVersion.replace(".", "_")}`; + } + + let generatorId = "other"; + // Keep these in sync with mozilla central's Histograms.json. + const KNOWN_GENERATORS = [ + "acrobat distiller", + "acrobat pdfwriter", + "adobe livecycle", + "adobe pdf library", + "adobe photoshop", + "ghostscript", + "tcpdf", + "cairo", + "dvipdfm", + "dvips", + "pdftex", + "pdfkit", + "itext", + "prince", + "quarkxpress", + "mac os x", + "microsoft", + "openoffice", + "oracle", + "luradocument", + "pdf-xchange", + "antenna house", + "aspose.cells", + "fpdf", + ]; + if (info.Producer) { + const producer = info.Producer.toLowerCase(); + KNOWN_GENERATORS.some(function(generator) { + if (!producer.includes(generator)) { + return false; } - } + generatorId = generator.replace(/[ .\-]/g, "_"); + return true; + }); + } - if (pdfTitle) { - this.setTitle( - `${pdfTitle} - ${contentDispositionFilename || document.title}` - ); - } else if (contentDispositionFilename) { - this.setTitle(contentDispositionFilename); - } - - if (info.IsAcroFormPresent) { - console.warn("Warning: AcroForm/XFA is not supported"); - this.fallback(UNSUPPORTED_FEATURES.forms); - } - - if ( - typeof PDFJSDev === "undefined" || - PDFJSDev.test("MOZCENTRAL || GENERIC") - ) { - // Telemetry labels must be C++ variable friendly. - let versionId = "other"; - // Keep these in sync with mozilla central's Histograms.json. - const KNOWN_VERSIONS = [ - "1.0", - "1.1", - "1.2", - "1.3", - "1.4", - "1.5", - "1.6", - "1.7", - "1.8", - "1.9", - "2.0", - "2.1", - "2.2", - "2.3", - ]; - if (KNOWN_VERSIONS.includes(info.PDFFormatVersion)) { - versionId = `v${info.PDFFormatVersion.replace(".", "_")}`; - } - - let generatorId = "other"; - // Keep these in sync with mozilla central's Histograms.json. - const KNOWN_GENERATORS = [ - "acrobat distiller", - "acrobat pdfwriter", - "adobe livecycle", - "adobe pdf library", - "adobe photoshop", - "ghostscript", - "tcpdf", - "cairo", - "dvipdfm", - "dvips", - "pdftex", - "pdfkit", - "itext", - "prince", - "quarkxpress", - "mac os x", - "microsoft", - "openoffice", - "oracle", - "luradocument", - "pdf-xchange", - "antenna house", - "aspose.cells", - "fpdf", - ]; - if (info.Producer) { - const producer = info.Producer.toLowerCase(); - KNOWN_GENERATORS.some(function(generator) { - if (!producer.includes(generator)) { - return false; - } - generatorId = generator.replace(/[ .\-]/g, "_"); - return true; - }); - } - - let formType = null; - if (info.IsAcroFormPresent) { - formType = info.IsXFAPresent ? "xfa" : "acroform"; - } - this.externalServices.reportTelemetry({ - type: "documentInfo", - version: versionId, - generator: generatorId, - formType, - }); - } + let formType = null; + if (info.IsAcroFormPresent) { + formType = info.IsXFAPresent ? "xfa" : "acroform"; + } + this.externalServices.reportTelemetry({ + type: "documentInfo", + version: versionId, + generator: generatorId, + formType, }); + } }, /** From b9add6509932c903b5c14d84556ef3a6551e5414 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 5 Apr 2020 10:59:10 +0200 Subject: [PATCH 3/4] Move the initialization of "auto print" out of `PDFViewerApplication.load` Over time, with more and more API-functionality added, the `PDFViewerApplication.load` method has become quite large and complex. In an attempt to improve the current situation somewhat, this patch moves the fetching and initialization of "auto print" out into its own (private) helper method instead. --- web/app.js | 91 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/web/app.js b/web/app.js index f0e64db3c..ab26f567b 100644 --- a/web/app.js +++ b/web/app.js @@ -1185,46 +1185,8 @@ const PDFViewerApplication = { }); }); - pagesPromise.then(async () => { - const [openAction, javaScript] = await Promise.all([ - openActionPromise, - pdfDocument.getJavaScript(), - ]); - let triggerAutoPrint = false; - - if (openAction && openAction.action === "Print") { - triggerAutoPrint = true; - } - if (javaScript) { - javaScript.some(js => { - if (!js) { - // Don't warn/fallback for empty JavaScript actions. - return false; - } - console.warn("Warning: JavaScript is not supported"); - this.fallback(UNSUPPORTED_FEATURES.javaScript); - return true; - }); - - if (!triggerAutoPrint) { - // Hack to support auto printing. - for (const js of javaScript) { - if (js && AutoPrintRegExp.test(js)) { - triggerAutoPrint = true; - break; - } - } - } - } - - if (!this.supportsPrinting) { - return; - } - if (triggerAutoPrint) { - setTimeout(function() { - window.print(); - }); - } + pagesPromise.then(() => { + this._initializeAutoPrint(pdfDocument, openActionPromise); }); onePageRendered.then(() => { @@ -1240,6 +1202,55 @@ const PDFViewerApplication = { this._initializeMetadata(pdfDocument); }, + /** + * @private + */ + async _initializeAutoPrint(pdfDocument, openActionPromise) { + const [openAction, javaScript] = await Promise.all([ + openActionPromise, + pdfDocument.getJavaScript(), + ]); + + if (pdfDocument !== this.pdfDocument) { + return; // The document was closed while the auto print data resolved. + } + let triggerAutoPrint = false; + + if (openAction && openAction.action === "Print") { + triggerAutoPrint = true; + } + if (javaScript) { + javaScript.some(js => { + if (!js) { + // Don't warn/fallback for empty JavaScript actions. + return false; + } + console.warn("Warning: JavaScript is not supported"); + this.fallback(UNSUPPORTED_FEATURES.javaScript); + return true; + }); + + if (!triggerAutoPrint) { + // Hack to support auto printing. + for (const js of javaScript) { + if (js && AutoPrintRegExp.test(js)) { + triggerAutoPrint = true; + break; + } + } + } + } + + if (!this.supportsPrinting) { + return; + } + if (triggerAutoPrint) { + setTimeout(function() { + window.print(); + }); + } + }, + /** * @private */ From 9ef58347ed3b640db1d4397a716b247688dd628a Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 5 Apr 2020 11:15:21 +0200 Subject: [PATCH 4/4] A couple of small improvements of the `PDFViewerApplication.{_initializeMetadata, _initializePdfHistory}` methods - Use template strings when printing document/viewer information in `_initializeMetadata`, since the old format feels overly verbose. Also, get the WebGL state from the `BaseViewer` instance[1] rather than the `AppOptions`. Since the `AppOptions` value could theoretically have been changed (by the user) after the viewer components were initialized, it seems much more useful to print the *actual* value that'll be used during rendering. - Change `_initializePdfHistory` to actually do the "is embedded"-check first, in accordance with the comment and given that the "disableHistory" option usually shouldn't be set. --- [1] Admittedly reaching into the `BaseViewer` instance and just grabbing the value perhaps isn't a great approach overall, but given that the WebGL-backend isn't even on by default this probably doesn't matter too much. --- web/app.js | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/web/app.js b/web/app.js index ab26f567b..5ddfd8adf 100644 --- a/web/app.js +++ b/web/app.js @@ -1270,19 +1270,10 @@ const PDFViewerApplication = { // Provides some basic debug information console.log( - "PDF " + - pdfDocument.fingerprint + - " [" + - info.PDFFormatVersion + - " " + - (info.Producer || "-").trim() + - " / " + - (info.Creator || "-").trim() + - "]" + - " (PDF.js: " + - (version || "-") + - (AppOptions.get("enableWebGL") ? " [WebGL]" : "") + - ")" + `PDF ${pdfDocument.fingerprint} [${info.PDFFormatVersion} ` + + `${(info.Producer || "-").trim()} / ${(info.Creator || "-").trim()}] ` + + `(PDF.js: ${version || "-"}` + + `${this.pdfViewer.enableWebGL ? " [WebGL]" : ""})` ); let pdfTitle; @@ -1444,7 +1435,7 @@ const PDFViewerApplication = { * @private */ _initializePdfHistory({ fingerprint, viewOnLoad, initialDest = null }) { - if (AppOptions.get("disableHistory") || this.isViewerEmbedded) { + if (this.isViewerEmbedded || AppOptions.get("disableHistory")) { // The browsing history is only enabled when the viewer is standalone, // i.e. not when it is embedded in a web page. return;