Skip to content

Commit

Permalink
[Editor] Avoid to focus an existing editor when enabling the layer
Browse files Browse the repository at this point in the history
It fixes #18911.
  • Loading branch information
calixteman committed Dec 11, 2024
1 parent fbac4ab commit f3dea83
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 1 deletion.
10 changes: 9 additions & 1 deletion src/display/editor/annotation_editor_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class AnnotationEditorLayer {

#isDisabling = false;

#isEnabling = false;

#drawingAC = null;

#focusedElement = null;
Expand Down Expand Up @@ -229,6 +231,7 @@ class AnnotationEditorLayer {
* editor creation.
*/
async enable() {
this.#isEnabling = true;
this.div.tabIndex = 0;
this.togglePointerEvents(true);
const annotationElementIds = new Set();
Expand All @@ -242,6 +245,7 @@ class AnnotationEditorLayer {
}

if (!this.#annotationLayer) {
this.#isEnabling = false;
return;
}

Expand All @@ -262,6 +266,7 @@ class AnnotationEditorLayer {
this.addOrRebuild(editor);
editor.enableEditing();
}
this.#isEnabling = false;
}

/**
Expand Down Expand Up @@ -508,7 +513,10 @@ class AnnotationEditorLayer {

// The editor will be correctly moved into the DOM (see fixAndSetPosition).
editor.fixAndSetPosition();
editor.onceAdded();
if (!this.#isEnabling) {
// When enabling the layer we don't want to focus the added editor.
editor.onceAdded();
}
this.#uiManager.addToAnnotationStorage(editor);
editor._reportTelemetry(editor.telemetryInitialData);
}
Expand Down
56 changes: 56 additions & 0 deletions test/integration/highlight_editor_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
waitForAnnotationModeChanged,
waitForSelectedEditor,
waitForSerialized,
waitForTimeout,
} from "./test_utils.mjs";
import { fileURLToPath } from "url";
import fs from "fs";
Expand Down Expand Up @@ -2613,4 +2614,59 @@ describe("Highlight Editor", () => {
);
});
});

describe("Highlight mustn't trigger a scroll when edited", () => {
let pages;

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

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

it("must check that there is no scroll because of focus", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await switchToHighlight(page);

const page4Selector = ".page[data-page-number = '4']";
const page5Selector = ".page[data-page-number = '5']";
await scrollIntoView(page, page4Selector);
await page.waitForSelector(`${page5Selector} .annotationEditorLayer`);

// When moving to page 3, the highlight editor mustn't be focused (it
// was causing a scroll to page 5).
// So here we're waiting a bit and checking that the page is still 3.
// eslint-disable-next-line no-restricted-syntax
await waitForTimeout(100);

// Get the length of the intersection between two ranges.
const inter = ([a, b], [c, d]) =>
d < a || b < c ? 0 : Math.min(b, d) - Math.max(a, c);

const page4Rect = await getRect(page, page4Selector);
const page5Rect = await getRect(page, page5Selector);
const viewportRect = await getRect(page, "#viewerContainer");
const viewportRange = [
viewportRect.y,
viewportRect.y + viewportRect.height,
];

const interPage4 = inter(
[page4Rect.y, page4Rect.y + page4Rect.height],
viewportRange
);
const interPage5 = inter(
[page5Rect.y, page5Rect.y + page5Rect.height],
viewportRange
);
expect(interPage4)
.withContext(`In ${browserName}`)
.toBeGreaterThan(0.5 * interPage5);
})
);
});
});
});
1 change: 1 addition & 0 deletions test/pdfs/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -689,3 +689,4 @@
!inks.pdf
!inks_basic.pdf
!issue19182.pdf
!issue18911.pdf
Binary file added test/pdfs/issue18911.pdf
Binary file not shown.

0 comments on commit f3dea83

Please sign in to comment.