Skip to content

Commit

Permalink
Merge pull request #14023 from Snuffleupagus/AnnotationLayer-getEleme…
Browse files Browse the repository at this point in the history
…ntsByName

Re-factor `document.getElementsByName` lookups in the AnnotationLayer (issue 14003)
  • Loading branch information
Snuffleupagus committed Sep 23, 2021
2 parents 8dc22f4 + 386acf5 commit c914e9f
Show file tree
Hide file tree
Showing 9 changed files with 245 additions and 78 deletions.
114 changes: 81 additions & 33 deletions src/display/annotation_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { AnnotationStorage } from "./annotation_storage.js";
import { ColorConverters } from "../shared/scripting_utils.js";

const DEFAULT_TAB_INDEX = 1000;
const GetElementsByNameSet = new WeakSet();

/**
* @typedef {Object} AnnotationElementParameters
Expand All @@ -50,6 +51,7 @@ const DEFAULT_TAB_INDEX = 1000;
* @property {Object} svgFactory
* @property {boolean} [enableScripting]
* @property {boolean} [hasJSActions]
* @property {Object} [fieldObjects]
* @property {Object} [mouseState]
*/

Expand Down Expand Up @@ -159,6 +161,7 @@ class AnnotationElement {
this.annotationStorage = parameters.annotationStorage;
this.enableScripting = parameters.enableScripting;
this.hasJSActions = parameters.hasJSActions;
this._fieldObjects = parameters.fieldObjects;
this._mouseState = parameters.mouseState;

if (isRenderable) {
Expand Down Expand Up @@ -363,6 +366,51 @@ class AnnotationElement {
unreachable("Abstract method `AnnotationElement.render` called");
}

/**
* @private
* @returns {Array}
*/
_getElementsByName(name, skipId = null) {
const fields = [];

if (this._fieldObjects) {
const fieldObj = this._fieldObjects[name];
if (fieldObj) {
for (const { page, id, exportValues } of fieldObj) {
if (page === -1) {
continue;
}
if (id === skipId) {
continue;
}
const exportValue =
typeof exportValues === "string" ? exportValues : null;

const domElement = document.getElementById(id);
if (domElement && !GetElementsByNameSet.has(domElement)) {
warn(`_getElementsByName - element not allowed: ${id}`);
continue;
}
fields.push({ id, exportValue, domElement });
}
}
return fields;
}
// Fallback to a regular DOM lookup, to ensure that the standalone
// viewer components won't break.
for (const domElement of document.getElementsByName(name)) {
const { id, exportValue } = domElement;
if (id === skipId) {
continue;
}
if (!GetElementsByNameSet.has(domElement)) {
continue;
}
fields.push({ id, exportValue, domElement });
}
return fields;
}

static get platform() {
const platform = typeof navigator !== "undefined" ? navigator.platform : "";

Expand Down Expand Up @@ -687,13 +735,14 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {

setPropertyOnSiblings(base, key, value, keyInStorage) {
const storage = this.annotationStorage;
for (const element of document.getElementsByName(base.name)) {
if (element !== base) {
element[key] = value;
const data = Object.create(null);
data[keyInStorage] = value;
storage.setValue(element.getAttribute("id"), data);
for (const element of this._getElementsByName(
base.name,
/* skipId = */ base.id
)) {
if (element.domElement) {
element.domElement[key] = value;
}
storage.setValue(element.id, { [keyInStorage]: value });
}
}

Expand Down Expand Up @@ -728,6 +777,9 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
element.type = "text";
element.setAttribute("value", textContent);
}
GetElementsByNameSet.add(element);
element.disabled = this.data.readOnly;
element.name = this.data.fieldName;
element.tabIndex = DEFAULT_TAB_INDEX;

elementData.userValue = textContent;
Expand Down Expand Up @@ -900,9 +952,6 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
element.addEventListener("blur", blurListener);
}

element.disabled = this.data.readOnly;
element.name = this.data.fieldName;

if (this.data.maxLen !== null) {
element.maxLength = this.data.maxLen;
}
Expand Down Expand Up @@ -978,6 +1027,7 @@ class CheckboxWidgetAnnotationElement extends WidgetAnnotationElement {
this.container.className = "buttonWidgetAnnotation checkBox";

const element = document.createElement("input");
GetElementsByNameSet.add(element);
element.disabled = data.readOnly;
element.type = "checkbox";
element.name = data.fieldName;
Expand All @@ -988,19 +1038,14 @@ class CheckboxWidgetAnnotationElement extends WidgetAnnotationElement {
element.setAttribute("exportValue", data.exportValue);
element.tabIndex = DEFAULT_TAB_INDEX;

element.addEventListener("change", function (event) {
const name = event.target.name;
const checked = event.target.checked;
for (const checkbox of document.getElementsByName(name)) {
if (checkbox !== event.target) {
checkbox.checked =
checked &&
checkbox.getAttribute("exportValue") === data.exportValue;
storage.setValue(
checkbox.parentNode.getAttribute("data-annotation-id"),
{ value: false }
);
element.addEventListener("change", event => {
const { name, checked } = event.target;
for (const checkbox of this._getElementsByName(name, /* skipId = */ id)) {
const curChecked = checked && checkbox.exportValue === data.exportValue;
if (checkbox.domElement) {
checkbox.domElement.checked = curChecked;
}
storage.setValue(checkbox.id, { value: curChecked });
}
storage.setValue(id, { value: checked });
});
Expand Down Expand Up @@ -1057,6 +1102,7 @@ class RadioButtonWidgetAnnotationElement extends WidgetAnnotationElement {
}

const element = document.createElement("input");
GetElementsByNameSet.add(element);
element.disabled = data.readOnly;
element.type = "radio";
element.name = data.fieldName;
Expand All @@ -1066,26 +1112,26 @@ class RadioButtonWidgetAnnotationElement extends WidgetAnnotationElement {
element.setAttribute("id", id);
element.tabIndex = DEFAULT_TAB_INDEX;

element.addEventListener("change", function (event) {
const { target } = event;
for (const radio of document.getElementsByName(target.name)) {
if (radio !== target) {
storage.setValue(radio.getAttribute("id"), { value: false });
}
element.addEventListener("change", event => {
const { name, checked } = event.target;
for (const radio of this._getElementsByName(name, /* skipId = */ id)) {
storage.setValue(radio.id, { value: false });
}
storage.setValue(id, { value: target.checked });
storage.setValue(id, { value: checked });
});

if (this.enableScripting && this.hasJSActions) {
const pdfButtonValue = data.buttonValue;
element.addEventListener("updatefromsandbox", jsEvent => {
const actions = {
value(event) {
value: event => {
const checked = pdfButtonValue === event.detail.value;
for (const radio of document.getElementsByName(event.target.name)) {
const radioId = radio.getAttribute("id");
radio.checked = radioId === id && checked;
storage.setValue(radioId, { value: radio.checked });
for (const radio of this._getElementsByName(event.target.name)) {
const curChecked = checked && radio.id === id;
if (radio.domElement) {
radio.domElement.checked = curChecked;
}
storage.setValue(radio.id, { value: curChecked });
}
},
};
Expand Down Expand Up @@ -1158,6 +1204,7 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
const fontSizeStyle = `calc(${fontSize}px * var(--zoom-factor))`;

const selectElement = document.createElement("select");
GetElementsByNameSet.add(selectElement);
selectElement.disabled = this.data.readOnly;
selectElement.name = this.data.fieldName;
selectElement.setAttribute("id", id);
Expand Down Expand Up @@ -2090,6 +2137,7 @@ class AnnotationLayer {
parameters.annotationStorage || new AnnotationStorage(),
enableScripting: parameters.enableScripting,
hasJSActions: parameters.hasJSActions,
fieldObjects: parameters.fieldObjects,
mouseState: parameters.mouseState || { isDown: false },
});
if (element.isRenderable) {
Expand Down
11 changes: 7 additions & 4 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1017,9 +1017,9 @@ class PDFDocumentProxy {
}

/**
* @returns {Promise<Array<Object> | null>} A promise that is resolved with an
* {Array<Object>} containing /AcroForm field data for the JS sandbox,
* or `null` when no field data is present in the PDF file.
* @returns {Promise<Object<string, Array<Object>> | null>} A promise that is
* resolved with an {Object} containing /AcroForm field data for the JS
* sandbox, or `null` when no field data is present in the PDF file.
*/
getFieldObjects() {
return this._transport.getFieldObjects();
Expand Down Expand Up @@ -2480,6 +2480,7 @@ class WorkerTransport {
Promise.all(waitOn).then(() => {
this.commonObjs.clear();
this.fontLoader.clear();
this._getFieldObjectsPromise = null;
this._hasJSActionsPromise = null;

if (this._networkStream) {
Expand Down Expand Up @@ -2921,7 +2922,8 @@ class WorkerTransport {
}

getFieldObjects() {
return this.messageHandler.sendWithPromise("GetFieldObjects", null);
return (this._getFieldObjectsPromise ||=
this.messageHandler.sendWithPromise("GetFieldObjects", null));
}

hasJSActions() {
Expand Down Expand Up @@ -3050,6 +3052,7 @@ class WorkerTransport {
if (!keepLoadedFonts) {
this.fontLoader.clear();
}
this._getFieldObjectsPromise = null;
this._hasJSActionsPromise = null;
}

Expand Down
95 changes: 95 additions & 0 deletions test/integration/annotation_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,98 @@ describe("Text widget", () => {
});
});
});

describe("Annotation and storage", () => {
describe("issue14023.pdf", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait("issue14023.pdf", "[data-annotation-id='64R']");
});

afterAll(async () => {
await closePages(pages);
});

it("must let checkboxes with the same name behave like radio buttons", async () => {
const text1 = "hello world!";
const text2 = "!dlrow olleh";
await Promise.all(
pages.map(async ([browserName, page]) => {
// Text field.
await page.type("#\\36 4R", text1);
// Checkbox.
await page.click("[data-annotation-id='65R']");
// Radio.
await page.click("[data-annotation-id='67R']");

for (const [pageNumber, textId, checkId, radio1Id, radio2Id] of [
[2, "#\\31 8R", "#\\31 9R", "#\\32 1R", "#\\32 0R"],
[5, "#\\32 3R", "#\\32 4R", "#\\32 2R", "#\\32 5R"],
]) {
await page.evaluate(n => {
window.document
.querySelectorAll(`[data-page-number="${n}"][class="page"]`)[0]
.scrollIntoView();
}, pageNumber);

// Need to wait to have a displayed text input.
await page.waitForSelector(textId, {
timeout: 0,
});

const text = await page.$eval(textId, el => el.value);
expect(text).withContext(`In ${browserName}`).toEqual(text1);

let checked = await page.$eval(checkId, el => el.checked);
expect(checked).toEqual(true);

checked = await page.$eval(radio1Id, el => el.checked);
expect(checked).toEqual(false);

checked = await page.$eval(radio2Id, el => el.checked);
expect(checked).toEqual(false);
}

// Change data on page 5 and check that other pages changed.
// Text field.
await page.type("#\\32 3R", text2);
// Checkbox.
await page.click("[data-annotation-id='24R']");
// Radio.
await page.click("[data-annotation-id='25R']");

for (const [pageNumber, textId, checkId, radio1Id, radio2Id] of [
[1, "#\\36 4R", "#\\36 5R", "#\\36 7R", "#\\36 8R"],
[2, "#\\31 8R", "#\\31 9R", "#\\32 1R", "#\\32 0R"],
]) {
await page.evaluate(n => {
window.document
.querySelectorAll(`[data-page-number="${n}"][class="page"]`)[0]
.scrollIntoView();
}, pageNumber);

// Need to wait to have a displayed text input.
await page.waitForSelector(textId, {
timeout: 0,
});

const text = await page.$eval(textId, el => el.value);
expect(text)
.withContext(`In ${browserName}`)
.toEqual(text2 + text1);

let checked = await page.$eval(checkId, el => el.checked);
expect(checked).toEqual(false);

checked = await page.$eval(radio1Id, el => el.checked);
expect(checked).toEqual(false);

checked = await page.$eval(radio2Id, el => el.checked);
expect(checked).toEqual(false);
}
})
);
});
});
});
1 change: 1 addition & 0 deletions test/pdfs/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
!issue11651.pdf
!issue11878.pdf
!issue13916.pdf
!issue14023.pdf
!bad-PageLabels.pdf
!decodeACSuccessive.pdf
!filled-background.pdf
Expand Down
Binary file added test/pdfs/issue14023.pdf
Binary file not shown.
Loading

0 comments on commit c914e9f

Please sign in to comment.