From 9af89381cd0458e0c772b11a4658c86a98839031 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Tue, 6 Dec 2022 16:16:24 +0100 Subject: [PATCH] [Editor] Add a very basic and incomplete workaround for issue #15780 The main issue is due to the fact that an editor's parent can be null when we want to serialize it and that lead to an exception which break all the saving/printing process. So this incomplete patch fixes only the saving/printing issue but not the underlying problem (i.e. having a null parent) and doesn't bring that much complexity, so it should help to uplift it the next Firefox release. --- src/display/editor/annotation_editor_layer.js | 4 +- src/display/editor/editor.js | 7 + src/display/editor/freetext.js | 4 + src/display/editor/ink.js | 4 + test/integration/freetext_editor_spec.js | 133 +++++++++++++++--- test/integration/test_utils.js | 18 +++ 6 files changed, 147 insertions(+), 23 deletions(-) diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index 6d5fea76c..f330514a4 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -246,7 +246,7 @@ class AnnotationEditorLayer { this.attach(editor); editor.pageIndex = this.pageIndex; editor.parent?.detach(editor); - editor.parent = this; + editor.setParent(this); if (editor.div && editor.isAttachedToDOM) { editor.div.remove(); this.div.append(editor.div); @@ -521,8 +521,8 @@ class AnnotationEditorLayer { for (const editor of this.#editors.values()) { this.#accessibilityManager?.removePointerInTextLayer(editor.contentDiv); editor.isAttachedToDOM = false; + editor.setParent(null); editor.div.remove(); - editor.parent = null; } this.div = null; this.#editors.clear(); diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index 60a1fcfb3..4900ec989 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -68,6 +68,8 @@ class AnnotationEditor { this.rotation = this.parent.viewport.rotation; this.isAttachedToDOM = false; + + this._serialized = undefined; } static get _defaultLineColor() { @@ -78,6 +80,11 @@ class AnnotationEditor { ); } + setParent(parent) { + this._serialized = !parent ? this.serialize() : undefined; + this.parent = parent; + } + /** * This editor will be behind the others. */ diff --git a/src/display/editor/freetext.js b/src/display/editor/freetext.js index fd7cc1f47..6042b9028 100644 --- a/src/display/editor/freetext.js +++ b/src/display/editor/freetext.js @@ -478,6 +478,10 @@ class FreeTextEditor extends AnnotationEditor { /** @inheritdoc */ serialize() { + if (this._serialized !== undefined) { + return this._serialized; + } + if (this.isEmpty()) { return null; } diff --git a/src/display/editor/ink.js b/src/display/editor/ink.js index 81d4129db..434d49e13 100644 --- a/src/display/editor/ink.js +++ b/src/display/editor/ink.js @@ -1058,6 +1058,10 @@ class InkEditor extends AnnotationEditor { /** @inheritdoc */ serialize() { + if (this._serialized !== undefined) { + return this._serialized; + } + if (this.isEmpty()) { return null; } diff --git a/test/integration/freetext_editor_spec.js b/test/integration/freetext_editor_spec.js index 0fea39fbd..37f904bd5 100644 --- a/test/integration/freetext_editor_spec.js +++ b/test/integration/freetext_editor_spec.js @@ -19,6 +19,8 @@ const { getSelectedEditors, loadAndWait, waitForEvent, + waitForSelectedEditor, + waitForStorageEntries, } = require("./test_utils.js"); const copyPaste = async page => { @@ -49,23 +51,6 @@ describe("Editor", () => { await closePages(pages); }); - const waitForStorageEntries = async (page, nEntries) => { - await page.waitForFunction( - n => - window.PDFViewerApplication.pdfDocument.annotationStorage.size === n, - {}, - nEntries - ); - }; - - const waitForSelected = async (page, selector) => { - await page.waitForFunction( - sel => document.querySelector(sel).classList.contains("selectedEditor"), - {}, - selector - ); - }; - it("must write a string in a FreeText editor", async () => { await Promise.all( pages.map(async ([browserName, page]) => { @@ -98,7 +83,7 @@ describe("Editor", () => { editorRect.y + 2 * editorRect.height ); - await waitForSelected(page, getEditorSelector(0)); + await waitForSelectedEditor(page, getEditorSelector(0)); await waitForStorageEntries(page, 1); const content = await page.$eval(getEditorSelector(0), el => @@ -123,7 +108,7 @@ describe("Editor", () => { editorRect.y + editorRect.height / 2 ); - await waitForSelected(page, getEditorSelector(0)); + await waitForSelectedEditor(page, getEditorSelector(0)); await copyPaste(page); await waitForStorageEntries(page, 2); @@ -199,7 +184,7 @@ describe("Editor", () => { editorRect.y + editorRect.height / 2 ); - await waitForSelected(page, getEditorSelector(3)); + await waitForSelectedEditor(page, getEditorSelector(3)); await copyPaste(page); let hasEditor = await page.evaluate(sel => { @@ -335,7 +320,7 @@ describe("Editor", () => { editorRect.y + editorRect.height / 2 ); - await waitForSelected(page, getEditorSelector(8)); + await waitForSelectedEditor(page, getEditorSelector(8)); expect(await getSelectedEditors(page)) .withContext(`In ${browserName}`) @@ -512,4 +497,110 @@ describe("Editor", () => { } }); }); + + describe("FreeText (bugs)", () => { + let pages; + + beforeAll(async () => { + pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer"); + }); + + afterAll(async () => { + await closePages(pages); + }); + + it("must serialize invisible annotations", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.click("#editorFreeText"); + let currentId = 0; + const expected = []; + const oneToFourteen = [...new Array(14).keys()].map(x => x + 1); + + for (const pageNumber of oneToFourteen) { + const pageSelector = `.page[data-page-number = "${pageNumber}"]`; + + await page.evaluate(selector => { + const element = window.document.querySelector(selector); + element.scrollIntoView(); + }, pageSelector); + + const annotationLayerSelector = `${pageSelector} > .annotationEditorLayer`; + await page.waitForSelector(annotationLayerSelector, { + visible: true, + timeout: 0, + }); + await page.waitForTimeout(50); + if (![1, 14].includes(pageNumber)) { + continue; + } + + const rect = await page.$eval(annotationLayerSelector, el => { + // With Chrome something is wrong when serializing a DomRect, + // hence we extract the values and just return them. + const { x, y } = el.getBoundingClientRect(); + return { x, y }; + }); + + const data = `Hello PDF.js World !! on page ${pageNumber}`; + expected.push(data); + await page.mouse.click(rect.x + 100, rect.y + 100); + await page.type(`${getEditorSelector(currentId)} .internal`, data); + + const editorRect = await page.$eval( + getEditorSelector(currentId), + el => { + const { x, y, width, height } = el.getBoundingClientRect(); + return { + x, + y, + width, + height, + }; + } + ); + + // Commit. + await page.mouse.click( + editorRect.x, + editorRect.y + 2 * editorRect.height + ); + + await waitForSelectedEditor(page, getEditorSelector(currentId)); + await waitForStorageEntries(page, currentId + 1); + + const content = await page.$eval(getEditorSelector(currentId), el => + el.innerText.trimEnd() + ); + expect(content).withContext(`In ${browserName}`).toEqual(data); + + currentId += 1; + await page.waitForTimeout(10); + } + + const serialize = proprName => + page.evaluate( + name => + [ + ...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.values(), + ].map(x => x[name]), + proprName + ); + + expect(await serialize("value")) + .withContext(`In ${browserName}`) + .toEqual(expected); + expect(await serialize("fontSize")) + .withContext(`In ${browserName}`) + .toEqual([10, 10]); + expect(await serialize("color")) + .withContext(`In ${browserName}`) + .toEqual([ + [0, 0, 0], + [0, 0, 0], + ]); + }) + ); + }); + }); }); diff --git a/test/integration/test_utils.js b/test/integration/test_utils.js index c50938d87..59e087164 100644 --- a/test/integration/test_utils.js +++ b/test/integration/test_utils.js @@ -100,3 +100,21 @@ async function waitForEvent(page, eventName, timeout = 30000) { ]); } exports.waitForEvent = waitForEvent; + +const waitForStorageEntries = async (page, nEntries) => { + await page.waitForFunction( + n => window.PDFViewerApplication.pdfDocument.annotationStorage.size === n, + {}, + nEntries + ); +}; +exports.waitForStorageEntries = waitForStorageEntries; + +const waitForSelectedEditor = async (page, selector) => { + await page.waitForFunction( + sel => document.querySelector(sel).classList.contains("selectedEditor"), + {}, + selector + ); +}; +exports.waitForSelectedEditor = waitForSelectedEditor;