Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for lazy merging dictionaries in annotations #12314

Closed
wants to merge 1 commit into from
Closed

Add support for lazy merging dictionaries in annotations #12314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions src/core/annotation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import {
} from "../shared/util.js";
import { Catalog, FileSpec, ObjectLoader } from "./obj.js";
import { Dict, isDict, isName, isRef, isStream, Name } from "./primitives.js";
import { getInheritableProperty, getLazyMergeDict } from "./core_utils.js";
import { ColorSpace } from "./colorspace.js";
import { getInheritableProperty } from "./core_utils.js";
import { OperatorList } from "./operator_list.js";
import { StringStream } from "./stream.js";
import { writeDict } from "./writer.js";
Expand Down Expand Up @@ -818,10 +818,29 @@ class WidgetAnnotation extends Annotation {
"";
const fieldType = getInheritableProperty({ dict, key: "FT" });
data.fieldType = isName(fieldType) ? fieldType.name : null;
this.fieldResources =
getInheritableProperty({ dict, key: "DR" }) ||
params.acroForm.get("DR") ||
Dict.empty;

const fallbackResources = params.acroForm.get("DR");
const defaultResources = getInheritableProperty({ dict, key: "DR" });

if (defaultResources && fallbackResources) {
const fontRes = defaultResources.get("Font");
const fallbackFontRes = fallbackResources.get("Font");
if (fontRes && fallbackFontRes) {
// A textfield can set a font which isn't in DR
// but in AcroForm::DR (#12294).
// So in this case we get the font in the fallback
// and we "fix" DR.
// We could merge DR with Acroform::DR but it'd lead
// to increase file size when saving.
defaultResources.set(
"Font",
getLazyMergeDict(fontRes, fallbackFontRes)
);
}
this.fieldResources = defaultResources;
} else {
this.fieldResources = defaultResources || fallbackResources || Dict.empty;
}

data.fieldFlags = getInheritableProperty({ dict, key: "Ff" });
if (!Number.isInteger(data.fieldFlags) || data.fieldFlags < 0) {
Expand Down
17 changes: 17 additions & 0 deletions src/core/core_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
/* eslint no-var: error */

import { assert, BaseException, warn } from "../shared/util.js";
import { isDict } from "./primitives.js";

function getLookupTableFactory(initializer) {
let lookup;
Expand Down Expand Up @@ -165,12 +166,28 @@ function isWhiteSpace(ch) {
return ch === 0x20 || ch === 0x09 || ch === 0x0d || ch === 0x0a;
}

function getLazyMergeDict(dict, fallback) {
assert(isDict(dict), "The two arguments must be a Dict");
dict.getRaw = function (key) {
let value = dict._map[key];
if (value === undefined) {
assert(isDict(fallback), "The two arguments must be a Dict");
assert(dict !== fallback, "The two arguments must be different");

dict._map[key] = value = fallback.getRaw(key);
}
return value;
};
return dict;
}

export {
getLookupTableFactory,
MissingDataException,
XRefEntryException,
XRefParseException,
getInheritableProperty,
getLazyMergeDict,
toRomanNumerals,
log2,
readInt8,
Expand Down
14 changes: 7 additions & 7 deletions src/core/primitives.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ var Dict = (function DictClosure() {

// automatically dereferences Ref objects
get(key1, key2, key3) {
let value = this._map[key1];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe these primitives are called a lot, so introducing extra function calls here might cause a performance regression. If we do this, it will have to be benchmarked carefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.
My first try was to use a Proxy object which should be fine to avoid any kind of perf regression but I didn't find a polyfill for it (according to MDN it isn't supported with IE) and so I gave up this solution.
But I guess that it should be possible now to use this solution since IE support has been removed.
Wdyt ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should aim for a much simpler solution, specifically for the case at hand here, rather than trying to extend a primitive such as Dict since there's other problems with this patch as far as I can tell.
E.g. the getLazyMergeDict function is mutating the provided dict parameter, which could very easily lead to issues elsewhere and even outright bugs if used without the out-most care (since you'd not expect that kind of behaviour).

Since I hack on PDF.js in my spare time, I've not yet had the time to look into all of this, but hopefully will later this week.

let value = this.getRaw(key1);
if (value === undefined && key2 !== undefined) {
value = this._map[key2];
value = this.getRaw(key2);
if (value === undefined && key3 !== undefined) {
value = this._map[key3];
value = this.getRaw(key3);
}
}
if (value instanceof Ref && this.xref) {
Expand All @@ -106,11 +106,11 @@ var Dict = (function DictClosure() {

// Same as get(), but returns a promise and uses fetchIfRefAsync().
async getAsync(key1, key2, key3) {
let value = this._map[key1];
let value = this.getRaw(key1);
if (value === undefined && key2 !== undefined) {
value = this._map[key2];
value = this.getRaw(key2);
if (value === undefined && key3 !== undefined) {
value = this._map[key3];
value = this.getRaw(key3);
}
}
if (value instanceof Ref && this.xref) {
Expand Down Expand Up @@ -161,7 +161,7 @@ var Dict = (function DictClosure() {
},

has: function Dict_has(key) {
return this._map[key] !== undefined;
return this.getRaw(key) !== undefined;
},

forEach: function Dict_forEach(callback) {
Expand Down
18 changes: 18 additions & 0 deletions test/unit/core_utils_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import { Dict, Ref } from "../../src/core/primitives.js";
import {
getInheritableProperty,
getLazyMergeDict,
isWhiteSpace,
log2,
toRomanNumerals,
Expand Down Expand Up @@ -211,4 +212,21 @@ describe("core_utils", function () {
expect(isWhiteSpace(undefined)).toEqual(false);
});
});

describe("getLazyMergeDict", function () {
it("gets values in a dict and lazily merge dictionaries", function () {
const base = new Dict(null);
base.set("a", 1);
const fallback = new Dict(null);
fallback.set("b", 2);

const dict = getLazyMergeDict(base, fallback);

expect(dict.getRaw("a")).toEqual(1);
expect(dict.getRaw("b")).toEqual(2);

fallback.set("b", 3);
expect(dict.getRaw("b")).toEqual(2);
});
});
});