[Editor] Avoid to use parent of editors in destroyed pages
This commit is contained in:
parent
cf5a1d60a6
commit
8ab4e2e6e7
@ -377,7 +377,10 @@ class AnnotationEditorLayer {
|
|||||||
editor.isAttachedToDOM = true;
|
editor.isAttachedToDOM = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The editor must have the right position before being moved in the DOM.
|
||||||
|
editor.fixAndSetPosition();
|
||||||
this.moveEditorInDOM(editor);
|
this.moveEditorInDOM(editor);
|
||||||
|
|
||||||
editor.onceAdded();
|
editor.onceAdded();
|
||||||
this.#uiManager.addToAnnotationStorage(editor);
|
this.#uiManager.addToAnnotationStorage(editor);
|
||||||
}
|
}
|
||||||
@ -675,6 +678,8 @@ class AnnotationEditorLayer {
|
|||||||
*/
|
*/
|
||||||
destroy() {
|
destroy() {
|
||||||
if (this.#uiManager.getActive()?.parent === this) {
|
if (this.#uiManager.getActive()?.parent === this) {
|
||||||
|
// We need to commit the current editor before destroying the layer.
|
||||||
|
this.#uiManager.commitOrRemove();
|
||||||
this.#uiManager.setActiveEditor(null);
|
this.#uiManager.setActiveEditor(null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -347,7 +347,14 @@ class AnnotationEditor {
|
|||||||
const [parentWidth, parentHeight] = this.parentDimensions;
|
const [parentWidth, parentHeight] = this.parentDimensions;
|
||||||
this.x += tx / parentWidth;
|
this.x += tx / parentWidth;
|
||||||
this.y += ty / parentHeight;
|
this.y += ty / parentHeight;
|
||||||
if (this.x < 0 || this.x > 1 || this.y < 0 || this.y > 1) {
|
if (this.parent && (this.x < 0 || this.x > 1 || this.y < 0 || this.y > 1)) {
|
||||||
|
// It's possible to not have a parent: for example, when the user is
|
||||||
|
// dragging all the selected editors but this one on a page which has been
|
||||||
|
// destroyed.
|
||||||
|
// It's why we need to check for it. In such a situation, it isn't really
|
||||||
|
// a problem to not find a new parent: it's something which is related to
|
||||||
|
// what the user is seeing, hence it depends on how pages are layed out.
|
||||||
|
|
||||||
// The element will be outside of its parent so change the parent.
|
// The element will be outside of its parent so change the parent.
|
||||||
const { x, y } = this.div.getBoundingClientRect();
|
const { x, y } = this.div.getBoundingClientRect();
|
||||||
if (this.parent.findNewParent(this, x, y)) {
|
if (this.parent.findNewParent(this, x, y)) {
|
||||||
|
@ -1565,6 +1565,8 @@ class AnnotationEditorUIManager {
|
|||||||
* Set up the drag session for moving the selected editors.
|
* Set up the drag session for moving the selected editors.
|
||||||
*/
|
*/
|
||||||
setUpDragSession() {
|
setUpDragSession() {
|
||||||
|
// Note: don't use any references to the editor's parent which can be null
|
||||||
|
// if the editor belongs to a destroyed page.
|
||||||
if (!this.hasSelection) {
|
if (!this.hasSelection) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@ -1575,7 +1577,7 @@ class AnnotationEditorUIManager {
|
|||||||
this.#draggingEditors.set(editor, {
|
this.#draggingEditors.set(editor, {
|
||||||
savedX: editor.x,
|
savedX: editor.x,
|
||||||
savedY: editor.y,
|
savedY: editor.y,
|
||||||
savedPageIndex: editor.parent.pageIndex,
|
savedPageIndex: editor.pageIndex,
|
||||||
newX: 0,
|
newX: 0,
|
||||||
newY: 0,
|
newY: 0,
|
||||||
newPageIndex: -1,
|
newPageIndex: -1,
|
||||||
@ -1596,14 +1598,14 @@ class AnnotationEditorUIManager {
|
|||||||
this.#draggingEditors = null;
|
this.#draggingEditors = null;
|
||||||
let mustBeAddedInUndoStack = false;
|
let mustBeAddedInUndoStack = false;
|
||||||
|
|
||||||
for (const [{ x, y, parent }, value] of map) {
|
for (const [{ x, y, pageIndex }, value] of map) {
|
||||||
value.newX = x;
|
value.newX = x;
|
||||||
value.newY = y;
|
value.newY = y;
|
||||||
value.newPageIndex = parent.pageIndex;
|
value.newPageIndex = pageIndex;
|
||||||
mustBeAddedInUndoStack ||=
|
mustBeAddedInUndoStack ||=
|
||||||
x !== value.savedX ||
|
x !== value.savedX ||
|
||||||
y !== value.savedY ||
|
y !== value.savedY ||
|
||||||
parent.pageIndex !== value.savedPageIndex;
|
pageIndex !== value.savedPageIndex;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!mustBeAddedInUndoStack) {
|
if (!mustBeAddedInUndoStack) {
|
||||||
|
@ -2344,4 +2344,89 @@ describe("FreeText Editor", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("FreeText on several pages", () => {
|
||||||
|
let pages;
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
|
||||||
|
});
|
||||||
|
|
||||||
|
afterAll(async () => {
|
||||||
|
await closePages(pages);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("must check that first annotation is selected without errors", async () => {
|
||||||
|
await Promise.all(
|
||||||
|
pages.map(async ([browserName, page]) => {
|
||||||
|
await page.click("#editorFreeText");
|
||||||
|
|
||||||
|
const page1Selector = `.page[data-page-number = "1"] > .annotationEditorLayer`;
|
||||||
|
let rect = await page.$eval(page1Selector, el => {
|
||||||
|
const { x, y } = el.getBoundingClientRect();
|
||||||
|
return { x, y };
|
||||||
|
});
|
||||||
|
await page.mouse.click(rect.x + 10, rect.y + 10);
|
||||||
|
await page.waitForTimeout(10);
|
||||||
|
await page.type(`${getEditorSelector(0)} .internal`, "Hello");
|
||||||
|
|
||||||
|
// Commit.
|
||||||
|
await page.keyboard.press("Escape");
|
||||||
|
await page.waitForTimeout(10);
|
||||||
|
|
||||||
|
// Go to the last page.
|
||||||
|
await page.keyboard.press("End");
|
||||||
|
await page.waitForTimeout(10);
|
||||||
|
|
||||||
|
const page14Selector = `.page[data-page-number = "14"] > .annotationEditorLayer`;
|
||||||
|
await page.waitForSelector(page14Selector, {
|
||||||
|
visible: true,
|
||||||
|
timeout: 0,
|
||||||
|
});
|
||||||
|
await page.waitForTimeout(10);
|
||||||
|
|
||||||
|
rect = await page.$eval(page14Selector, el => {
|
||||||
|
const { x, y } = el.getBoundingClientRect();
|
||||||
|
return { x, y };
|
||||||
|
});
|
||||||
|
await page.mouse.click(rect.x + 10, rect.y + 10);
|
||||||
|
await page.waitForTimeout(10);
|
||||||
|
await page.type(`${getEditorSelector(0)} .internal`, "World");
|
||||||
|
|
||||||
|
await page.keyboard.press("Escape");
|
||||||
|
await page.waitForTimeout(10);
|
||||||
|
|
||||||
|
for (let i = 0; i <= 13; i++) {
|
||||||
|
await page.keyboard.press("P");
|
||||||
|
await page.waitForTimeout(10);
|
||||||
|
}
|
||||||
|
|
||||||
|
await page.waitForSelector(getEditorSelector(0), {
|
||||||
|
visible: true,
|
||||||
|
timeout: 0,
|
||||||
|
});
|
||||||
|
await page.waitForTimeout(10);
|
||||||
|
|
||||||
|
rect = await page.$eval(getEditorSelector(0), el => {
|
||||||
|
const { x, y, width, height } = el.getBoundingClientRect();
|
||||||
|
return {
|
||||||
|
x,
|
||||||
|
y,
|
||||||
|
width,
|
||||||
|
height,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
await page.mouse.click(
|
||||||
|
rect.x + rect.width / 2,
|
||||||
|
rect.y + rect.height / 2
|
||||||
|
);
|
||||||
|
|
||||||
|
const content = await page.$eval(getEditorSelector(0), el =>
|
||||||
|
el.innerText.trimEnd()
|
||||||
|
);
|
||||||
|
expect(content).withContext(`In ${browserName}`).toEqual("Hello");
|
||||||
|
})
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
@ -53,6 +53,8 @@ exports.loadAndWait = (filename, selector, zoom, pageSetup) =>
|
|||||||
exports.closePages = pages =>
|
exports.closePages = pages =>
|
||||||
Promise.all(
|
Promise.all(
|
||||||
pages.map(async ([_, page]) => {
|
pages.map(async ([_, page]) => {
|
||||||
|
// Avoid to keep something from a previous test.
|
||||||
|
await page.evaluate(() => window.localStorage.clear());
|
||||||
await page.close();
|
await page.close();
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
|
Loading…
x
Reference in New Issue
Block a user