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

[Editor] Make sure that annotation positions in the DOM respect the visual order (bug 1992770)

Browse Source
This commit is contained in:
Calixte Denizet 2025-10-07 21:01:03 +02:00
parent d83cbb28a9
commit 41dea1e38b
5 changed files with 172 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/display/annotation_layer.js
View File

@ -186,7 +186,8 @@ class AnnotationElement {
this.parent = parameters.parent;
if (isRenderable) {
this.container = this._createContainer(ignoreBorder);
this.contentElement = this.container =
this._createContainer(ignoreBorder);
}
if (createQuadrilaterals) {
this._createQuadrilaterals();
@ -1008,6 +1009,7 @@ class LinkAnnotationElement extends AnnotationElement {
this.container.classList.add("linkAnnotation");
if (isBound) {
this.contentElement = link;
this.container.append(link);
}
@ -1517,6 +1519,7 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
element.hidden = true;
}
GetElementsByNameSet.add(element);
this.contentElement = element;
element.setAttribute("data-element-id", id);
element.disabled = this.data.readOnly;
@ -3070,7 +3073,7 @@ class FreeTextAnnotationElement extends AnnotationElement {
this.container.classList.add("freeTextAnnotation");
if (this.textContent) {
const content = document.createElement("div");
const content = (this.contentElement = document.createElement("div"));
content.classList.add("annotationTextContent");
content.setAttribute("role", "comment");
for (const line of this.textContent) {
@ -3787,7 +3790,7 @@ class AnnotationLayer {
}
async #appendElement(element, id, popupElements) {
const contentElement = element.firstChild || element;
const { contentElement, container } = element;
const annotationId = (contentElement.id = `${AnnotationPrefix}${id}`);
const ariaAttributes =
await this.#structTreeLayer?.getAriaAttributes(annotationId);
@ -3799,18 +3802,31 @@ class AnnotationLayer {
if (popupElements) {
// Set the popup just after the first element associated with the popup.
popupElements.at(-1).container.after(element);
popupElements.at(-1).container.after(container);
} else {
this.div.append(element);
this.#accessibilityManager?.moveElementInDOM(
this.div,
element,
contentElement,
/* isRemovable = */ false
);
this.#moveElementInDOM(container, contentElement);
}
}
#moveElementInDOM(container, contentElement) {
this.div.append(container);
this.#accessibilityManager?.moveElementInDOM(
this.div,
container,
contentElement,
/* isRemovable = */ false,
/* filter = */ node => node.nodeName === "SECTION",
/* inserter = */ (prevNode, node) => {
if (prevNode.nextElementSibling.nodeName === "BUTTON") {
// In case we have a comment button, insert after the button.
prevNode.nextElementSibling.after(node);
} else {
prevNode.after(node);
}
}
);
}
/**
* Render a new annotation layer with all annotation elements.
*
@ -3877,7 +3893,7 @@ class AnnotationLayer {
if (data.hidden) {
rendered.style.visibility = "hidden";
}
await this.#appendElement(rendered, data.id, elementParams.elements);
await this.#appendElement(element, data.id, elementParams.elements);
element.extraPopupElement?.popup?.renderCommentButton();
if (element._isEditable) {
@ -3913,8 +3929,8 @@ class AnnotationLayer {
if (!element.isRenderable) {
continue;
}
const rendered = element.render();
await this.#appendElement(rendered, data.id, null);
element.render();
await this.#appendElement(element, data.id, null);
}
}
@ -3999,18 +4015,32 @@ class AnnotationLayer {
linkService: this.#linkService,
annotationStorage: this.#annotationStorage,
});
const htmlElement = element.render();
div.append(htmlElement);
this.#accessibilityManager?.moveElementInDOM(
div,
htmlElement,
htmlElement,
/* isRemovable = */ false
);
const rendered = element.render();
rendered.id = `${AnnotationPrefix}${id}`;
this.#moveElementInDOM(rendered, rendered);
element.createOrUpdatePopup();
return element;
}
togglePointerEvents(enabled = false) {
this.div.classList.toggle("disabled", !enabled);
}
updateFakeAnnotations(editors) {
if (editors.length === 0) {
return;
}
// In order to ensure that the annotations are correctly moved in the DOM
// we need to make sure that this has been laid out.
window.requestAnimationFrame(() =>
setTimeout(() => {
for (const editor of editors) {
editor.updateFakeAnnotationElement(this);
}
}, 10)
);
}
/**
* @private
*/

6
src/display/editor/annotation_editor_layer.js
View File

@ -242,7 +242,7 @@ class AnnotationEditorLayer {
}
toggleAnnotationLayerPointerEvents(enabled = false) {
this.#annotationLayer?.div.classList.toggle("disabled", !enabled);
this.#annotationLayer?.togglePointerEvents(enabled);
}
get #allEditorsIterator() {
@ -354,13 +354,14 @@ class AnnotationEditorLayer {
}
const annotationLayer = this.#annotationLayer;
const needFakeAnnotation = [];
if (annotationLayer) {
const changedAnnotations = new Map();
const resetAnnotations = new Map();
for (const editor of this.#allEditorsIterator) {
editor.disableEditing();
if (!editor.annotationElementId) {
editor.updateFakeAnnotationElement(annotationLayer);
needFakeAnnotation.push(editor);
continue;
}
if (editor.serialize() !== null) {
@ -411,6 +412,7 @@ class AnnotationEditorLayer {
}
this.disableTextSelection();
this.toggleAnnotationLayerPointerEvents(true);
annotationLayer?.updateFakeAnnotations(needFakeAnnotation);
this.#isDisabling = false;
}

44
test/integration/annotation_spec.mjs
View File

@ -95,7 +95,7 @@ describe("Annotation highlight", () => {
pages.map(async ([browserName, page]) => {
for (const i of [23, 22, 14]) {
await page.click(getAnnotationSelector(`${i}R`));
await page.waitForSelector(`#pdfjs_internal_id_${i}R:focus`);
await page.waitForSelector(`#pdfjs_internal_id_${i}R:focus-within`);
}
})
);
@ -878,4 +878,46 @@ a dynamic compiler for JavaScript based on our`);
});
});
});
describe("Annotation order in the DOM", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"comments.pdf",
".page[data-page-number='1'] .annotationLayer #pdfjs_internal_id_661R"
);
});
afterEach(async () => {
await closePages(pages);
});
it("must check that annotations are in the visual order", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const sectionIds = await page.evaluate(() =>
[
...document.querySelectorAll(
".page[data-page-number='1'] .annotationLayer > section:not(.popupAnnotation)"
),
].map(el => el.id.split("_").pop())
);
expect(sectionIds)
.withContext(`In ${browserName}`)
.toEqual([
"612R",
"693R",
"687R",
"690R",
"713R",
"673R",
"613R",
"680R",
"661R",
]);
})
);
});
});
});

61
test/integration/comment_spec.mjs
View File

@ -737,4 +737,65 @@ describe("Comment", () => {
);
});
});
describe("Annotations order in reading mode", () => {
let pages;
beforeEach(async () => {
pages = await loadAndWait(
"comments.pdf",
".annotationEditorLayer",
"page-fit",
null,
{ enableComment: true }
);
});
afterEach(async () => {
await closePages(pages);
});
it("must check that the annotations are in the right order", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await switchToHighlight(page);
await highlightSpan(
page,
1,
"method provides cheap inter-procedural type specialization, and an"
);
await editComment(page, getEditorSelector(9), "Hello world!");
await highlightSpan(page, 1, "Andreas Gal");
await editComment(page, getEditorSelector(10), "Hello world!");
await switchToHighlight(page, /* disable = */ true);
await page.waitForSelector(
".annotationLayer section:nth-child(4).editorAnnotation"
);
const sectionIds = await page.evaluate(() =>
[
...document.querySelectorAll(
".page[data-page-number='1'] .annotationLayer > section:not(.popupAnnotation)"
),
].map(el => el.id.split("_").pop())
);
expect(sectionIds).withContext(`In ${browserName}`).toEqual([
"612R",
"693R",
"10", // shortcut for pdfjs_internal_id_pdfjs_internal_editor_10
"687R",
"690R",
"713R",
"9", // shortcut for pdfjs_internal_id_pdfjs_internal_editor_9
"673R",
"613R",
"680R",
"661R",
]);
})
);
});
});
});

19
web/text_accessibility.js
View File

@ -222,7 +222,14 @@ class TextAccessibilityManager {
* @param {HTMLDivElement} element
* @returns {string|null} The id in the struct tree if any.
*/
moveElementInDOM(container, element, contentElement, isRemovable) {
moveElementInDOM(
container,
element,
contentElement,
isRemovable,
filter,
inserter
) {
const id = this.addPointerInTextLayer(contentElement, isRemovable);
if (!container.hasChildNodes()) {
@ -231,25 +238,23 @@ class TextAccessibilityManager {
}
const children = Array.from(container.childNodes).filter(
node => node !== element
node => node !== element && (!filter || filter(node))
);
if (children.length === 0) {
return id;
}
const elementToCompare = contentElement || element;
const index = binarySearchFirstItem(
children,
node =>
TextAccessibilityManager.#compareElementPositions(
elementToCompare,
node
) < 0
TextAccessibilityManager.#compareElementPositions(element, node) < 0
);
if (index === 0) {
children[0].before(element);
} else if (inserter) {
inserter(children[index - 1], element);
} else {
children[index - 1].after(element);
}