Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Replace element ids with custom attributes for Widget-annotations (issue 15056)

Browse Source 
We want to avoid adding regular `id`s to Annotation-elements, since that means that they become "linkable" through the URL hash in a way that's not supported/intended. This could end up clashing with "named destinations", and that could easily lead to bugs; see issue 11499 and PR 11503 for some context.

Rather than using `id`s, we'll instead use a *custom* `data-element-id` attribute such that it's still possible to access the Annotation-elements directly.
Unfortunately these changes required updating most of the integration-tests, and to reduce the amount of repeated code a couple of helper functions were added.
This commit is contained in:
Jonas Jenwald 2022-06-17 22:01:20 +02:00
parent 3ca8d2c4f9
commit 03757d82b7
5 changed files with 425 additions and 384 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/display/annotation_layer.js
View File

@ -522,7 +522,9 @@ class AnnotationElement {
const exportValue =
typeof exportValues === "string" ? exportValues : null;
const domElement = document.getElementById(id);
const domElement = document.querySelector(
`[data-element-id="${id}"]`
);
if (domElement && !GetElementsByNameSet.has(domElement)) {
warn(`_getElementsByName - element not allowed: ${id}`);
continue;
@ -570,7 +572,7 @@ class LinkAnnotationElement extends AnnotationElement {
render() {
const { data, linkService } = this;
const link = document.createElement("a");
link.setAttribute("id", data.id);
link.setAttribute("data-element-id", data.id);
let isBound = false;
if (data.url) {
@ -775,8 +777,12 @@ class LinkAnnotationElement extends AnnotationElement {
default:
continue;
}
const domElement = document.getElementById(id);
if (!domElement || !GetElementsByNameSet.has(domElement)) {
const domElement = document.querySelector(`[data-element-id="${id}"]`);
if (!domElement) {
continue;
} else if (!GetElementsByNameSet.has(domElement)) {
warn(`_bindResetFormAction - element not allowed: ${id}`);
continue;
}
domElement.dispatchEvent(new Event("resetform"));
@ -989,7 +995,7 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
});
const textContent = storedData.formattedValue || storedData.value || "";
const elementData = {
userValue: null,
userValue: textContent,
formattedValue: null,
valueOnFocus: "",
};
@ -1009,13 +1015,12 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
}
}
GetElementsByNameSet.add(element);
element.setAttribute("data-element-id", id);
element.disabled = this.data.readOnly;
element.name = this.data.fieldName;
element.tabIndex = DEFAULT_TAB_INDEX;
elementData.userValue = textContent;
element.setAttribute("id", id);
this._setRequired(element, this.data.required);
element.addEventListener("input", event => {
@ -1262,6 +1267,8 @@ class CheckboxWidgetAnnotationElement extends WidgetAnnotationElement {
const element = document.createElement("input");
GetElementsByNameSet.add(element);
element.setAttribute("data-element-id", id);
element.disabled = data.readOnly;
this._setRequired(element, this.data.required);
element.type = "checkbox";
@ -1269,7 +1276,6 @@ class CheckboxWidgetAnnotationElement extends WidgetAnnotationElement {
if (value) {
element.setAttribute("checked", true);
}
element.setAttribute("id", id);
element.setAttribute("exportValue", data.exportValue);
element.tabIndex = DEFAULT_TAB_INDEX;
@ -1346,6 +1352,8 @@ class RadioButtonWidgetAnnotationElement extends WidgetAnnotationElement {
const element = document.createElement("input");
GetElementsByNameSet.add(element);
element.setAttribute("data-element-id", id);
element.disabled = data.readOnly;
this._setRequired(element, this.data.required);
element.type = "radio";
@ -1353,7 +1361,6 @@ class RadioButtonWidgetAnnotationElement extends WidgetAnnotationElement {
if (value) {
element.setAttribute("checked", true);
}
element.setAttribute("id", id);
element.tabIndex = DEFAULT_TAB_INDEX;
element.addEventListener("change", event => {
@ -1463,10 +1470,11 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
const selectElement = document.createElement("select");
GetElementsByNameSet.add(selectElement);
selectElement.setAttribute("data-element-id", id);
selectElement.disabled = this.data.readOnly;
this._setRequired(selectElement, this.data.required);
selectElement.name = this.data.fieldName;
selectElement.setAttribute("id", id);
selectElement.tabIndex = DEFAULT_TAB_INDEX;
let addAnEmptyEntry = this.data.combo && this.data.options.length > 0;

105
test/integration/annotation_spec.js
View File

@ -13,7 +13,12 @@
* limitations under the License.
*/
const { closePages, loadAndWait } = require("./test_utils.js");
const {
closePages,
getSelector,
getQuerySelector,
loadAndWait,
} = require("./test_utils.js");
describe("Annotation highlight", () => {
describe("annotation-highlight.pdf", () => {
@ -108,18 +113,14 @@ describe("Text widget", () => {
const base = "hello world";
await Promise.all(
pages.map(async ([browserName, page]) => {
await page.type("#\\32 5R", base);
await page.waitForFunction(
`document.querySelector("#\\\\32 4R").value !== ""`
);
await page.waitForFunction(
`document.querySelector("#\\\\32 6R").value !== ""`
);
await page.type(getSelector("25R"), base);
await page.waitForFunction(`${getQuerySelector("24R")}.value !== ""`);
await page.waitForFunction(`${getQuerySelector("26R")}.value !== ""`);
let text = await page.$eval("#\\32 4R", el => el.value);
let text = await page.$eval(getSelector("24R"), el => el.value);
expect(text).withContext(`In ${browserName}`).toEqual(base);
text = await page.$eval("#\\32 6R", el => el.value);
text = await page.$eval(getSelector("26R"), el => el.value);
expect(text).withContext(`In ${browserName}`).toEqual(base);
})
);
@ -145,15 +146,15 @@ describe("Annotation and storage", () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
// Text field.
await page.type("#\\36 4R", text1);
await page.type(getSelector("64R"), 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"],
[2, "18R", "19R", "21R", "20R"],
[5, "23R", "24R", "22R", "25R"],
]) {
await page.evaluate(n => {
window.document
@ -162,34 +163,37 @@ describe("Annotation and storage", () => {
}, pageNumber);
// Need to wait to have a displayed text input.
await page.waitForSelector(textId, {
await page.waitForSelector(getSelector(textId), {
timeout: 0,
});
const text = await page.$eval(textId, el => el.value);
const text = await page.$eval(getSelector(textId), el => el.value);
expect(text).withContext(`In ${browserName}`).toEqual(text1);
let checked = await page.$eval(checkId, el => el.checked);
let checked = await page.$eval(
getSelector(checkId),
el => el.checked
);
expect(checked).toEqual(true);
checked = await page.$eval(radio1Id, el => el.checked);
checked = await page.$eval(getSelector(radio1Id), el => el.checked);
expect(checked).toEqual(false);
checked = await page.$eval(radio2Id, el => el.checked);
checked = await page.$eval(getSelector(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);
await page.type(getSelector("23R"), 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"],
[1, "64R", "65R", "67R", "68R"],
[2, "18R", "19R", "21R", "20R"],
]) {
await page.evaluate(n => {
window.document
@ -198,22 +202,25 @@ describe("Annotation and storage", () => {
}, pageNumber);
// Need to wait to have a displayed text input.
await page.waitForSelector(textId, {
await page.waitForSelector(getSelector(textId), {
timeout: 0,
});
const text = await page.$eval(textId, el => el.value);
const text = await page.$eval(getSelector(textId), el => el.value);
expect(text)
.withContext(`In ${browserName}`)
.toEqual(text2 + text1);
let checked = await page.$eval(checkId, el => el.checked);
let checked = await page.$eval(
getSelector(checkId),
el => el.checked
);
expect(checked).toEqual(false);
checked = await page.$eval(radio1Id, el => el.checked);
checked = await page.$eval(getSelector(radio1Id), el => el.checked);
expect(checked).toEqual(false);
checked = await page.$eval(radio2Id, el => el.checked);
checked = await page.$eval(getSelector(radio2Id), el => el.checked);
expect(checked).toEqual(false);
}
})
@ -238,8 +245,8 @@ describe("ResetForm action", () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const base = "hello world";
for (let i = 3; i <= 7; i++) {
await page.type(`#\\36 ${i}R`, base);
for (let i = 63; i <= 67; i++) {
await page.type(getSelector(`${i}R`), base);
}
const selectors = [69, 71, 75].map(
@ -249,36 +256,34 @@ describe("ResetForm action", () => {
await page.click(selector);
}
await page.select("#\\37 8R", "b");
await page.select("#\\38 1R", "f");
await page.select(getSelector("78R"), "b");
await page.select(getSelector("81R"), "f");
await page.click("[data-annotation-id='82R']");
await page.waitForFunction(
`document.querySelector("#\\\\36 3R").value === ""`
);
await page.waitForFunction(`${getQuerySelector("63R")}.value === ""`);
for (let i = 3; i <= 8; i++) {
const text = await page.$eval(`#\\36 ${i}R`, el => el.value);
for (let i = 63; i <= 68; i++) {
const text = await page.$eval(getSelector(`${i}R`), el => el.value);
expect(text).withContext(`In ${browserName}`).toEqual("");
}
const ids = [69, 71, 72, 73, 74, 75, 76, 77];
for (const id of ids) {
const checked = await page.$eval(
`#\\3${Math.floor(id / 10)} ${id % 10}R`,
getSelector(`${id}R`),
el => el.checked
);
expect(checked).withContext(`In ${browserName}`).toEqual(false);
}
let selected = await page.$eval(
`#\\37 8R [value="a"]`,
`${getSelector("78R")} [value="a"]`,
el => el.selected
);
expect(selected).withContext(`In ${browserName}`).toEqual(true);
selected = await page.$eval(
`#\\38 1R [value="d"]`,
`${getSelector("81R")} [value="d"]`,
el => el.selected
);
expect(selected).withContext(`In ${browserName}`).toEqual(true);
@ -290,8 +295,8 @@ describe("ResetForm action", () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const base = "hello world";
for (let i = 3; i <= 8; i++) {
await page.type(`#\\36 ${i}R`, base);
for (let i = 63; i <= 68; i++) {
await page.type(getSelector(`${i}R`), base);
}
const selectors = [69, 71, 72, 73, 75].map(
@ -301,24 +306,22 @@ describe("ResetForm action", () => {
await page.click(selector);
}
await page.select("#\\37 8R", "b");
await page.select("#\\38 1R", "f");
await page.select(getSelector("78R"), "b");
await page.select(getSelector("81R"), "f");
await page.click("[data-annotation-id='84R']");
await page.waitForFunction(
`document.querySelector("#\\\\36 3R").value === ""`
);
await page.waitForFunction(`${getQuerySelector("63R")}.value === ""`);
for (let i = 3; i <= 8; i++) {
for (let i = 63; i <= 68; i++) {
const expected = (i - 3) % 2 === 0 ? "" : base;
const text = await page.$eval(`#\\36 ${i}R`, el => el.value);
const text = await page.$eval(getSelector(`${i}R`), el => el.value);
expect(text).withContext(`In ${browserName}`).toEqual(expected);
}
let ids = [69, 72, 73, 74, 76, 77];
for (const id of ids) {
const checked = await page.$eval(
`#\\3${Math.floor(id / 10)} ${id % 10}R`,
getSelector(`${id}R`),
el => el.checked
);
expect(checked)
@ -329,20 +332,20 @@ describe("ResetForm action", () => {
ids = [71, 75];
for (const id of ids) {
const checked = await page.$eval(
`#\\3${Math.floor(id / 10)} ${id % 10}R`,
getSelector(`${id}R`),
el => el.checked
);
expect(checked).withContext(`In ${browserName}`).toEqual(true);
}
let selected = await page.$eval(
`#\\37 8R [value="a"]`,
`${getSelector("78R")} [value="a"]`,
el => el.selected
);
expect(selected).withContext(`In ${browserName}`).toEqual(true);
selected = await page.$eval(
`#\\38 1R [value="f"]`,
`${getSelector("81R")} [value="f"]`,
el => el.selected
);
expect(selected).withContext(`In ${browserName}`).toEqual(true);

655
test/integration/scripting_spec.js
View File

File diff suppressed because it is too large Load Diff

15
test/integration/test_utils.js
View File

@ -58,3 +58,18 @@ exports.clearInput = async (page, selector) => {
await page.keyboard.up("Control");
await page.keyboard.press("Backspace");
};
function getSelector(id) {
return `[data-element-id="${id}"]`;
}
exports.getSelector = getSelector;
function getQuerySelector(id) {
return `document.querySelector('${getSelector(id)}')`;
}
exports.getQuerySelector = getQuerySelector;
function getComputedStyleSelector(id) {
return `getComputedStyle(${getQuerySelector(id)})`;
}
exports.getComputedStyleSelector = getComputedStyleSelector;

4
web/pdf_scripting_manager.js
View File

@ -351,7 +351,9 @@ class PDFScriptingManager {
const ids = siblings ? [id, ...siblings] : [id];
for (const elementId of ids) {
const element = document.getElementById(elementId);
const element = document.querySelector(
`[data-element-id="${elementId}"]`
);
if (element) {
element.dispatchEvent(new CustomEvent("updatefromsandbox", { detail }));
} else {