Skip to content

Commit

Permalink
[Editor] Add a very basic and incomplete workaround for issue mozilla…
Browse files Browse the repository at this point in the history
…#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.
  • Loading branch information
calixteman committed Dec 6, 2022
1 parent cdd39ec commit 323a9ad
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/display/editor/annotation_editor_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 7 additions & 0 deletions src/display/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ class AnnotationEditor {
this.rotation = this.parent.viewport.rotation;

this.isAttachedToDOM = false;

this._serialized = undefined;
}

static get _defaultLineColor() {
Expand All @@ -78,6 +80,11 @@ class AnnotationEditor {
);
}

setParent(parent) {
this._serialized = !parent ? this.serialize() : undefined;
this.parent = parent;
}

/**
* This editor will be behind the others.
*/
Expand Down
4 changes: 4 additions & 0 deletions src/display/editor/freetext.js
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,10 @@ class FreeTextEditor extends AnnotationEditor {

/** @inheritdoc */
serialize() {
if (this._serialized !== undefined) {
return this._serialized;
}

if (this.isEmpty()) {
return null;
}
Expand Down
4 changes: 4 additions & 0 deletions src/display/editor/ink.js
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,10 @@ class InkEditor extends AnnotationEditor {

/** @inheritdoc */
serialize() {
if (this._serialized !== undefined) {
return this._serialized;
}

if (this.isEmpty()) {
return null;
}
Expand Down
135 changes: 113 additions & 22 deletions test/integration/freetext_editor_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const {
getSelectedEditors,
loadAndWait,
waitForEvent,
waitForSelectedEditor,
waitForStorageEntries,
} = require("./test_utils.js");

const copyPaste = async page => {
Expand All @@ -37,7 +39,7 @@ const copyPaste = async page => {
await promise;
};

describe("Editor", () => {
fdescribe("Editor", () => {
describe("FreeText", () => {
let pages;

Expand All @@ -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]) => {
Expand Down Expand Up @@ -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 =>
Expand All @@ -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);

Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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}`)
Expand Down Expand Up @@ -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],
]);
})
);
});
});
});
18 changes: 18 additions & 0 deletions test/integration/test_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

0 comments on commit 323a9ad

Please sign in to comment.