Skip to content

Commit

Permalink
[Editor] Don't try to use an non-existing canvas when rendering an in…
Browse files Browse the repository at this point in the history
…visible existing stamp editor

It fixes mozilla#19239.

When the canvas isn't existing the editor has no image: it's fine because the editor is invisible.
Once it's made visible, the canvas is set when the annotation layer has been rendered.
  • Loading branch information
calixteman committed Jan 10, 2025
1 parent f1166f4 commit 60acc3e
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 15 deletions.
16 changes: 16 additions & 0 deletions src/display/annotation_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3301,6 +3301,22 @@ class AnnotationLayer {
} else {
firstChild.after(canvas);
}

const editableAnnotation = this.#editableAnnotations.get(id);
if (!editableAnnotation) {
continue;
}
if (editableAnnotation._hasNoCanvas) {
// The canvas wasn't available when the annotation was created.
this._annotationEditorUIManager?.setMissingCanvas(
id,
element.id,
canvas
);
editableAnnotation._hasNoCanvas = false;
} else {
editableAnnotation.canvas = canvas;
}
}
this.#annotationCanvasMap.clear();
}
Expand Down
60 changes: 46 additions & 14 deletions src/display/editor/stamp.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ class StampEditor extends AnnotationEditor {

#canvas = null;

#missingCanvas = false;

#resizeTimeoutId = null;

#isSvg = false;
Expand Down Expand Up @@ -352,7 +354,8 @@ class StampEditor extends AnnotationEditor {
this.#bitmap ||
this.#bitmapUrl ||
this.#bitmapFile ||
this.#bitmapId
this.#bitmapId ||
this.#missingCanvas
);
}

Expand All @@ -379,10 +382,12 @@ class StampEditor extends AnnotationEditor {

this.addAltTextButton();

if (this.#bitmap) {
this.#createCanvas();
} else {
this.#getBitmap();
if (!this.#missingCanvas) {
if (this.#bitmap) {
this.#createCanvas();
} else {
this.#getBitmap();
}
}

if (this.width && !this.annotationElementId) {
Expand All @@ -401,6 +406,22 @@ class StampEditor extends AnnotationEditor {
return this.div;
}

setCanvas(annotationElementId, canvas) {
const { id: bitmapId, bitmap } = this._uiManager.imageManager.getFromCanvas(
annotationElementId,
canvas
);
canvas.remove();
if (bitmapId && this._uiManager.imageManager.isValidId(bitmapId)) {
this.#bitmapId = bitmapId;
if (bitmap) {
this.#bitmap = bitmap;
}
this.#missingCanvas = false;
this.#createCanvas();
}
}

/** @inheritdoc */
_onResized() {
// We used a CSS-zoom during the resizing, but now it's resized we can
Expand Down Expand Up @@ -752,20 +773,28 @@ class StampEditor extends AnnotationEditor {
/** @inheritdoc */
static async deserialize(data, parent, uiManager) {
let initialData = null;
let missingCanvas = false;
if (data instanceof StampAnnotationElement) {
const {
data: { rect, rotation, id, structParent, popupRef },
container,
parent: {
page: { pageNumber },
},
canvas,
} = data;
const canvas = container.querySelector("canvas");
const imageData = uiManager.imageManager.getFromCanvas(
container.id,
canvas
);
canvas.remove();
let bitmapId, bitmap;
if (canvas) {
delete data.canvas;
({ id: bitmapId, bitmap } = uiManager.imageManager.getFromCanvas(
container.id,
canvas
));
canvas.remove();
} else {
missingCanvas = true;
data._hasNoCanvas = true;
}

// When switching to edit mode, we wait for the structure tree to be
// ready (see pdf_viewer.js), so it's fine to use getAriaAttributesSync.
Expand All @@ -776,8 +805,8 @@ class StampEditor extends AnnotationEditor {

initialData = data = {
annotationType: AnnotationEditorType.STAMP,
bitmapId: imageData.id,
bitmap: imageData.bitmap,
bitmapId,
bitmap,
pageIndex: pageNumber - 1,
rect: rect.slice(0),
rotation,
Expand All @@ -795,7 +824,10 @@ class StampEditor extends AnnotationEditor {
const editor = await super.deserialize(data, parent, uiManager);
const { rect, bitmap, bitmapUrl, bitmapId, isSvg, accessibilityData } =
data;
if (bitmapId && uiManager.imageManager.isValidId(bitmapId)) {
if (missingCanvas) {
uiManager.addMissingCanvas(data.id, editor);
editor.#missingCanvas = true;
} else if (bitmapId && uiManager.imageManager.isValidId(bitmapId)) {
editor.#bitmapId = bitmapId;
if (bitmap) {
editor.#bitmap = bitmap;
Expand Down
20 changes: 20 additions & 0 deletions src/display/editor/tools.js
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,8 @@ class AnnotationEditorUIManager {

#mainHighlightColorPicker = null;

#missingCanvases = null;

#mlManager = null;

#mode = AnnotationEditorType.NONE;
Expand Down Expand Up @@ -898,6 +900,7 @@ class AnnotationEditorUIManager {
this.#allLayers.clear();
this.#allEditors.clear();
this.#editorsToRescale.clear();
this.#missingCanvases?.clear();
this.#activeEditor = null;
this.#selectedEditors.clear();
this.#commandManager.destroy();
Expand Down Expand Up @@ -1711,6 +1714,10 @@ class AnnotationEditorUIManager {
this.#updateModeCapability.resolve();
}

isInEditingMode() {
return this.#mode !== AnnotationEditorType.NONE;
}

addNewEditorFromKeyboard() {
if (this.currentLayer.canCreateNewEmptyEditor()) {
this.currentLayer.addNewEditor();
Expand Down Expand Up @@ -2514,6 +2521,19 @@ class AnnotationEditorUIManager {
}
editor.renderAnnotationElement(annotation);
}

setMissingCanvas(annotationId, annotationElementId, canvas) {
const editor = this.#missingCanvases?.get(annotationId);
if (!editor) {
return;
}
editor.setCanvas(annotationElementId, canvas);
this.#missingCanvases.delete(annotationId);
}

addMissingCanvas(annotationId, editor) {
(this.#missingCanvases ||= new Map()).set(annotationId, editor);
}
}

export {
Expand Down
69 changes: 69 additions & 0 deletions test/integration/stamp_editor_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1745,4 +1745,73 @@ describe("Stamp Editor", () => {
);
});
});

describe("Switch to edit mode a pdf with an existing stamp annotation on an invisible and rendered page", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait("issue19239.pdf", ".annotationEditorLayer");
});

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

it("must move on the second page", async () => {
await Promise.all(
pages.map(async ([, page]) => {
const pageOneSelector = `.page[data-page-number = "1"]`;
const pageTwoSelector = `.page[data-page-number = "2"]`;
await scrollIntoView(page, pageTwoSelector);
await page.waitForSelector(pageOneSelector, {
visible: false,
});

await switchToStamp(page);
await scrollIntoView(page, pageOneSelector);
await page.waitForSelector(
`${pageOneSelector} .annotationEditorLayer canvas`,
{ visible: true }
);
})
);
});
});

describe("Switch to edit mode a pdf with an existing stamp annotation on an invisible and unrendered page", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait("issue19239.pdf", ".annotationEditorLayer");
});

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

it("must move on the last page", async () => {
await Promise.all(
pages.map(async ([, page]) => {
const twoToFourteen = Array.from(new Array(13).keys(), n => n + 2);
for (const pageNumber of twoToFourteen) {
const pageSelector = `.page[data-page-number = "${pageNumber}"]`;
await scrollIntoView(page, pageSelector);
}

await switchToStamp(page);

const thirteenToOne = Array.from(new Array(13).keys(), n => 13 - n);
for (const pageNumber of thirteenToOne) {
const pageSelector = `.page[data-page-number = "${pageNumber}"]`;
await scrollIntoView(page, pageSelector);
}

await page.waitForSelector(
`.page[data-page-number = "1"] .annotationEditorLayer canvas`,
{ visible: true }
);
})
);
});
});
});
1 change: 1 addition & 0 deletions test/pdfs/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -693,3 +693,4 @@
!issue19182.pdf
!issue18911.pdf
!issue19207.pdf
!issue19239.pdf
Binary file added test/pdfs/issue19239.pdf
Binary file not shown.
6 changes: 5 additions & 1 deletion web/pdf_page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,10 +588,14 @@ class PDFPageView {
}

toggleEditingMode(isEditing) {
// The page can be invisible, consequently there's no annotation layer and
// we can't know if there are editable annotations.
// So to avoid any issue when the page is rendered the #isEditing flag must
// be set.
this.#isEditing = isEditing;
if (!this.hasEditableAnnotations()) {
return;
}
this.#isEditing = isEditing;
this.reset({
keepAnnotationLayer: true,
keepAnnotationEditorLayer: true,
Expand Down

0 comments on commit 60acc3e

Please sign in to comment.