From 8e4ef6d89d399ac6103d4fc23baca15e83148a50 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 2 Oct 2022 11:26:08 +0200 Subject: [PATCH 1/2] [api-minor] Stop setting an `id` on the styleElement used with CSS font-loading This is yet another small piece of clean-up of the `FontLoader`-code, since we've not used this `id`-property for anything ever since PR 6571 (which landed almost seven years ago). Furthermore, by default we're also not even using that code-path now since the Font Loading API will always be used when available. *Please note:* This is tagged `[api-minor]` since it's technically observable from the outside, however no user ought to be directly interacting with these CSS font rules. --- src/display/api.js | 1 - src/display/font_loader.js | 12 ++++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index e449ea319..a0b2156c9 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2281,7 +2281,6 @@ class WorkerTransport { this.loadingTask = loadingTask; this.commonObjs = new PDFObjects(); this.fontLoader = new FontLoader({ - docId: loadingTask.docId, onUnsupportedFeature: this._onUnsupportedFeature.bind(this), ownerDocument: params.ownerDocument, styleElement: params.styleElement, diff --git a/src/display/font_loader.js b/src/display/font_loader.js index 185e42585..5cb508da7 100644 --- a/src/display/font_loader.js +++ b/src/display/font_loader.js @@ -25,12 +25,10 @@ import { class FontLoader { constructor({ - docId, onUnsupportedFeature, ownerDocument = globalThis.document, styleElement = null, // For testing only. }) { - this.docId = docId; this._onUnsupportedFeature = onUnsupportedFeature; this._document = ownerDocument; @@ -52,15 +50,13 @@ class FontLoader { } insertRule(rule) { - let styleElement = this.styleElement; - if (!styleElement) { - styleElement = this.styleElement = this._document.createElement("style"); - styleElement.id = `PDFJS_FONT_STYLE_TAG_${this.docId}`; + if (!this.styleElement) { + this.styleElement = this._document.createElement("style"); this._document.documentElement .getElementsByTagName("head")[0] - .append(styleElement); + .append(this.styleElement); } - const styleSheet = styleElement.sheet; + const styleSheet = this.styleElement.sheet; styleSheet.insertRule(rule, styleSheet.cssRules.length); } From fe5d9b4b6a3e14e54b6eaed95ee62d3875a892a3 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 2 Oct 2022 11:57:15 +0200 Subject: [PATCH 2/2] Remove duplicated `destroy`-calls in the "custom ownerDocument" unit-tests Given that `PDFDocumentProxy.destroy` is nothing but an alias for `PDFDocumentLoadingTask.destroy` calling both methods is obviously not useful. --- test/unit/custom_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/custom_spec.js b/test/unit/custom_spec.js index a0773c369..ae33736ec 100644 --- a/test/unit/custom_spec.js +++ b/test/unit/custom_spec.js @@ -172,7 +172,7 @@ describe("custom ownerDocument", function () { expect(style).toBeFalsy(); expect(ownerDocument.fonts.size).toBeGreaterThanOrEqual(1); expect(Array.from(ownerDocument.fonts).find(checkFont)).toBeTruthy(); - await doc.destroy(); + await loadingTask.destroy(); CanvasFactory.destroy(canvasAndCtx); expect(ownerDocument.fonts.size).toBe(0); @@ -204,7 +204,7 @@ describe("custom ownerDocument", function () { const style = elements.find(element => element.tagName === "style"); expect(style.sheet.cssRules.length).toBeGreaterThanOrEqual(1); expect(style.sheet.cssRules.find(checkFontFaceRule)).toBeTruthy(); - await doc.destroy(); + await loadingTask.destroy(); CanvasFactory.destroy(canvasAndCtx); expect(style.remove.called).toBe(true);