From 0eb1433c788d7546e280aeda323c8cdb1c0efadc Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 1 Apr 2021 15:19:45 +0200 Subject: [PATCH] [api-minor] Change the format of the `fontName`-property, in `defaultAppearanceData`, on Annotation-instances (PR 12831 follow-up) Currently the `fontName`-property contains an actual /Name-instance, which is a problem given that its fallback value is an empty string; see https://github.com/mozilla/pdf.js/blob/ca7f546828603d15ac0975f6131669321bfccceb/src/core/default_appearance.js#L35 The reason that this is a problem can be seen in https://github.com/mozilla/pdf.js/blob/ca7f546828603d15ac0975f6131669321bfccceb/src/core/primitives.js#L30-L34, since an empty string short-circuits the cache. Essentially, in PDF documents, a /Name-instance cannot be empty and the way that the `DefaultAppearanceEvaluator` does things is unfortunately not entirely correct. Hence the `fontName`-property is changed to instead contain a string, rather than a /Name-instance, which simplifies the code overall. *Please note:* I'm tagging this patch with "[api-minor]", since PR 12831 is included in the current pre-release (although we're not using the `fontName`-property in the display-layer). --- src/core/annotation.js | 14 +++++++------- src/core/default_appearance.js | 12 ++++++------ src/core/evaluator.js | 8 ++------ test/unit/default_appearance_spec.js | 7 +++---- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/core/annotation.js b/src/core/annotation.js index e96d0881f..1896e8989 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -1470,7 +1470,7 @@ class WidgetAnnotation extends Annotation { const { fontName, fontSize } = this.data.defaultAppearanceData; await evaluator.handleSetFont( this._fieldResources.mergedResources, - [fontName, fontSize], + [fontName && Name.get(fontName), fontSize], /* fontRef = */ null, operatorList, task, @@ -1565,26 +1565,26 @@ class WidgetAnnotation extends Annotation { acroFormResources, } = this._fieldResources; - const fontNameStr = + const fontName = this.data.defaultAppearanceData && - this.data.defaultAppearanceData.fontName.name; - if (!fontNameStr) { + this.data.defaultAppearanceData.fontName; + if (!fontName) { return localResources || Dict.empty; } for (const resources of [localResources, appearanceResources]) { if (resources instanceof Dict) { const localFont = resources.get("Font"); - if (localFont instanceof Dict && localFont.has(fontNameStr)) { + if (localFont instanceof Dict && localFont.has(fontName)) { return resources; } } } if (acroFormResources instanceof Dict) { const acroFormFont = acroFormResources.get("Font"); - if (acroFormFont instanceof Dict && acroFormFont.has(fontNameStr)) { + if (acroFormFont instanceof Dict && acroFormFont.has(fontName)) { const subFontDict = new Dict(xref); - subFontDict.set(fontNameStr, acroFormFont.getRaw(fontNameStr)); + subFontDict.set(fontName, acroFormFont.getRaw(fontName)); const subResourcesDict = new Dict(xref); subResourcesDict.set("Font", subFontDict); diff --git a/src/core/default_appearance.js b/src/core/default_appearance.js index 7f55229dc..4b2d9eb6e 100644 --- a/src/core/default_appearance.js +++ b/src/core/default_appearance.js @@ -13,11 +13,11 @@ * limitations under the License. */ -import { isName, Name } from "./primitives.js"; import { OPS, warn } from "../shared/util.js"; import { ColorSpace } from "./colorspace.js"; import { escapePDFName } from "./core_utils.js"; import { EvaluatorPreprocessor } from "./evaluator.js"; +import { Name } from "./primitives.js"; import { StringStream } from "./stream.js"; class DefaultAppearanceEvaluator extends EvaluatorPreprocessor { @@ -32,8 +32,8 @@ class DefaultAppearanceEvaluator extends EvaluatorPreprocessor { }; const result = { fontSize: 0, - fontName: Name.get(""), - fontColor: new Uint8ClampedArray([0, 0, 0]) /* black */, + fontName: "", + fontColor: /* black = */ new Uint8ClampedArray(3), }; try { @@ -51,8 +51,8 @@ class DefaultAppearanceEvaluator extends EvaluatorPreprocessor { switch (fn | 0) { case OPS.setFont: const [fontName, fontSize] = args; - if (isName(fontName)) { - result.fontName = fontName; + if (fontName instanceof Name) { + result.fontName = fontName.name; } if (typeof fontSize === "number" && fontSize > 0) { result.fontSize = fontSize; @@ -93,7 +93,7 @@ function createDefaultAppearance({ fontSize, fontName, fontColor }) { .map(c => (c / 255).toFixed(2)) .join(" ") + " rg"; } - return `/${escapePDFName(fontName.name)} ${fontSize} Tf ${colorCmd}`; + return `/${escapePDFName(fontName)} ${fontSize} Tf ${colorCmd}`; } export { createDefaultAppearance, parseDefaultAppearance }; diff --git a/src/core/evaluator.js b/src/core/evaluator.js index e09bd4b38..526f3fcbc 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -794,12 +794,8 @@ class PartialEvaluator { state, fallbackFontDict = null ) { - // TODO(mack): Not needed? - var fontName; - if (fontArgs) { - fontArgs = fontArgs.slice(); - fontName = fontArgs[0].name; - } + const fontName = + fontArgs && fontArgs[0] instanceof Name ? fontArgs[0].name : null; return this.loadFont(fontName, fontRef, resources, fallbackFontDict) .then(translated => { diff --git a/test/unit/default_appearance_spec.js b/test/unit/default_appearance_spec.js index a1caf26da..1a73db468 100644 --- a/test/unit/default_appearance_spec.js +++ b/test/unit/default_appearance_spec.js @@ -17,7 +17,6 @@ import { createDefaultAppearance, parseDefaultAppearance, } from "../../src/core/default_appearance.js"; -import { Name } from "../../src/core/primitives.js"; describe("Default appearance", function () { describe("parseDefaultAppearance and createDefaultAppearance", function () { @@ -25,7 +24,7 @@ describe("Default appearance", function () { const da = "/F1 12 Tf 0.10 0.20 0.30 rg"; const result = { fontSize: 12, - fontName: Name.get("F1"), + fontName: "F1", fontColor: new Uint8ClampedArray([26, 51, 76]), }; expect(parseDefaultAppearance(da)).toEqual(result); @@ -37,7 +36,7 @@ describe("Default appearance", function () { ) ).toEqual({ fontSize: 13, - fontName: Name.get("F2"), + fontName: "F2", fontColor: new Uint8ClampedArray([76, 51, 26]), }); }); @@ -47,7 +46,7 @@ describe("Default appearance", function () { "q Q 0.10 0.20 0.30 rg /F1 12 Tf q 0.30 0.20 0.10 rg /F2 13 Tf Q"; expect(parseDefaultAppearance(da)).toEqual({ fontSize: 12, - fontName: Name.get("F1"), + fontName: "F1", fontColor: new Uint8ClampedArray([26, 51, 76]), }); });