From 1450da4168854c59ac082026bb3360cef4ec6e78 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 16 Oct 2021 11:49:24 +0200 Subject: [PATCH 1/4] Fix a `xfaFaxtory` typo in the shadowing in the `PDFDocument.xfaFactory` getter With this typo the shadowing doesn't actually work, which causes these checks to be unnecessarily repeated. In this particular case it didn't have a significant performance impact, however we should definately fix this nonetheless. --- src/core/document.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/document.js b/src/core/document.js index 9262a0cbaa012..02161e2c13e56 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -892,7 +892,7 @@ class PDFDocument { const data = this.xfaData; return shadow(this, "xfaFactory", data ? new XFAFactory(data) : null); } - return shadow(this, "xfaFaxtory", null); + return shadow(this, "xfaFactory", null); } get isPureXfa() { From cd94a44ca1b02315300fbdf22e5db07e382e028d Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 16 Oct 2021 12:16:40 +0200 Subject: [PATCH 2/4] Remove some duplication in *simple* shadowed getters in `src/core/`-code In these cases there's no good reason, in my opinion, to duplicate the `shadow`-lines since that unnecessarily increases the risk of simple typos (see the previous patch). --- src/core/catalog.js | 18 ++++++++++-------- src/core/document.js | 28 ++++++++++++++++------------ 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/core/catalog.js b/src/core/catalog.js index 64c2143449238..2ace6fa402b9b 100644 --- a/src/core/catalog.js +++ b/src/core/catalog.js @@ -82,10 +82,11 @@ class Catalog { get version() { const version = this._catDict.get("Version"); - if (!isName(version)) { - return shadow(this, "version", null); - } - return shadow(this, "version", version.name); + return shadow( + this, + "version", + version instanceof Name ? version.name : null + ); } /** @@ -94,10 +95,11 @@ class Catalog { */ get needsRendering() { const needsRendering = this._catDict.get("NeedsRendering"); - if (!isBool(needsRendering)) { - return shadow(this, "needsRendering", false); - } - return shadow(this, "needsRendering", needsRendering); + return shadow( + this, + "needsRendering", + typeof needsRendering === "boolean" ? needsRendering : false + ); } get collection() { diff --git a/src/core/document.js b/src/core/document.js index 02161e2c13e56..21373afc2918e 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -263,12 +263,13 @@ class Page { } get xfaData() { - if (this.xfaFactory) { - return shadow(this, "xfaData", { - bbox: this.xfaFactory.getBoundingBox(this.pageIndex), - }); - } - return shadow(this, "xfaData", null); + return shadow( + this, + "xfaData", + this.xfaFactory + ? { bbox: this.xfaFactory.getBoundingBox(this.pageIndex) } + : null + ); } save(handler, task, annotationStorage) { @@ -784,11 +785,14 @@ class PDFDocument { } get numPages() { + let num = 0; if (this.xfaFactory) { - return shadow(this, "numPages", this.xfaFactory.numberPages); + num = this.xfaFactory.numberPages; + } else if (this.linearization) { + num = this.linearization.numPages; + } else { + num = this.catalog.numPages; } - const linearization = this.linearization; - const num = linearization ? linearization.numPages : this.catalog.numPages; return shadow(this, "numPages", num); } @@ -883,16 +887,16 @@ class PDFDocument { } get xfaFactory() { + let data; if ( this.pdfManager.enableXfa && this.catalog.needsRendering && this.formInfo.hasXfa && !this.formInfo.hasAcroForm ) { - const data = this.xfaData; - return shadow(this, "xfaFactory", data ? new XFAFactory(data) : null); + data = this.xfaData; } - return shadow(this, "xfaFactory", null); + return shadow(this, "xfaFactory", data ? new XFAFactory(data) : null); } get isPureXfa() { From 0e5348180efcb140c49aa131a3072574664b9364 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 16 Oct 2021 12:28:48 +0200 Subject: [PATCH 3/4] Fix the inconsistent return type of the `PDFDocument.isPureXfa` getter Also (slightly) simplifies a couple of small getters/methods related to the `XFAFactory`-instance. --- src/core/document.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/core/document.js b/src/core/document.js index 21373afc2918e..ab0d12cb2c792 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -900,14 +900,11 @@ class PDFDocument { } get isPureXfa() { - return this.xfaFactory && this.xfaFactory.isValid(); + return this.xfaFactory ? this.xfaFactory.isValid() : false; } get htmlForXfa() { - if (this.xfaFactory) { - return this.xfaFactory.getPages(); - } - return null; + return this.xfaFactory ? this.xfaFactory.getPages() : null; } async loadXfaImages() { @@ -1088,10 +1085,9 @@ class PDFDocument { } async serializeXfaData(annotationStorage) { - if (this.xfaFactory) { - return this.xfaFactory.serializeData(annotationStorage); - } - return null; + return this.xfaFactory + ? this.xfaFactory.serializeData(annotationStorage) + : null; } get formInfo() { From 004123007218a96784fc604dac89111b2c69fb6b Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 16 Oct 2021 12:35:12 +0200 Subject: [PATCH 4/4] Re-name the `XFAFactory.numberPages` getter to `XFAFactory.numPages` for consistency All other similar getters are called `numPages` throughout the code-base, and improved consistency should always be a good thing. --- src/core/document.js | 2 +- src/core/xfa/factory.js | 2 +- test/unit/xfa_tohtml_spec.js | 28 ++++++++++++++-------------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/core/document.js b/src/core/document.js index ab0d12cb2c792..4c77f10caed3a 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -787,7 +787,7 @@ class PDFDocument { get numPages() { let num = 0; if (this.xfaFactory) { - num = this.xfaFactory.numberPages; + num = this.xfaFactory.numPages; } else if (this.linearization) { num = this.linearization.numPages; } else { diff --git a/src/core/xfa/factory.js b/src/core/xfa/factory.js index fb4568aebf7f6..e21d07863490c 100644 --- a/src/core/xfa/factory.js +++ b/src/core/xfa/factory.js @@ -54,7 +54,7 @@ class XFAFactory { return this.dims[pageIndex]; } - get numberPages() { + get numPages() { if (!this.pages) { this._createPages(); } diff --git a/test/unit/xfa_tohtml_spec.js b/test/unit/xfa_tohtml_spec.js index e9ba898bbd2f2..da16e4bce9a07 100644 --- a/test/unit/xfa_tohtml_spec.js +++ b/test/unit/xfa_tohtml_spec.js @@ -86,7 +86,7 @@ describe("XFAFactory", function () { const factory = new XFAFactory({ "xdp:xdp": xml }); factory.setFonts([]); - expect(factory.numberPages).toEqual(2); + expect(factory.numPages).toEqual(2); const pages = factory.getPages(); const page1 = pages.children[0]; @@ -174,7 +174,7 @@ describe("XFAFactory", function () { `; const factory = new XFAFactory({ "xdp:xdp": xml }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); const field = searchHtmlNode(pages, "name", "img"); @@ -208,7 +208,7 @@ describe("XFAFactory", function () { `; const factory = new XFAFactory({ "xdp:xdp": xml }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); const page1 = pages.children[0]; @@ -263,7 +263,7 @@ describe("XFAFactory", function () { const factory = new XFAFactory({ "xdp:xdp": xml }); factory.setFonts([]); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); const table = searchHtmlNode( @@ -336,7 +336,7 @@ describe("XFAFactory", function () { `; const factory = new XFAFactory({ "xdp:xdp": xml }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); const field = searchHtmlNode(pages, "name", "input"); @@ -378,7 +378,7 @@ describe("XFAFactory", function () { `; const factory = new XFAFactory({ "xdp:xdp": xml }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); const field = searchHtmlNode(pages, "name", "input"); @@ -420,7 +420,7 @@ describe("XFAFactory", function () { `; const factory = new XFAFactory({ "xdp:xdp": xml }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); const field = searchHtmlNode(pages, "name", "input"); @@ -463,7 +463,7 @@ describe("XFAFactory", function () { `; const factory = new XFAFactory({ "xdp:xdp": xml }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); const field1 = searchHtmlNode(pages, "name", "input"); @@ -517,7 +517,7 @@ describe("XFAFactory", function () { `; const factory = new XFAFactory({ "xdp:xdp": xml }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); const field1 = searchHtmlNode(pages, "name", "input"); @@ -560,7 +560,7 @@ describe("XFAFactory", function () { // A valid, and complete, URL. factory = new XFAFactory({ "xdp:xdp": getXml("https://www.example.com/") }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); pages = factory.getPages(); a = searchHtmlNode(pages, "name", "a"); expect(a.value).toEqual("https://www.example.com/"); @@ -568,7 +568,7 @@ describe("XFAFactory", function () { // A valid, but incomplete, URL. factory = new XFAFactory({ "xdp:xdp": getXml("www.example.com/") }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); pages = factory.getPages(); a = searchHtmlNode(pages, "name", "a"); expect(a.value).toEqual("www.example.com/"); @@ -576,7 +576,7 @@ describe("XFAFactory", function () { // A valid email-address. factory = new XFAFactory({ "xdp:xdp": getXml("mailto:test@example.com") }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); pages = factory.getPages(); a = searchHtmlNode(pages, "name", "a"); expect(a.value).toEqual("mailto:test@example.com"); @@ -584,7 +584,7 @@ describe("XFAFactory", function () { // Not a valid URL. factory = new XFAFactory({ "xdp:xdp": getXml("qwerty/") }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); pages = factory.getPages(); a = searchHtmlNode(pages, "name", "a"); expect(a.value).toEqual("qwerty/"); @@ -635,7 +635,7 @@ describe("XFAFactory", function () { `; const factory = new XFAFactory({ "xdp:xdp": xml }); - expect(factory.numberPages).toEqual(1); + expect(factory.numPages).toEqual(1); const pages = factory.getPages(); let a = searchHtmlNode(pages, "name", "a");