Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Editor] Avoid to focus an existing editor when enabling the layer #19215

Merged
merged 1 commit into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 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,7 @@ class AnnotationEditorLayer {

// The editor will be correctly moved into the DOM (see fixAndSetPosition).
editor.fixAndSetPosition();
editor.onceAdded();
editor.onceAdded(/* focus = */ !this.#isEnabling);
this.#uiManager.addToAnnotationStorage(editor);
editor._reportTelemetry(editor.telemetryInitialData);
}
Expand Down
4 changes: 2 additions & 2 deletions src/display/editor/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ class DrawingEditor extends AnnotationEditor {
}

/** @inheritdoc */
onceAdded() {
onceAdded(focus) {
if (!this.annotationElementId) {
this.parent.addUndoableEditor(this);
}
Expand All @@ -354,7 +354,7 @@ class DrawingEditor extends AnnotationEditor {
this.#mustBeCommitted = false;
this.commit();
this.parent.setSelected(this);
if (this.isOnScreen) {
if (focus && this.isOnScreen) {
this.div.focus();
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/display/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -1367,8 +1367,9 @@ class AnnotationEditor {

/**
* Executed once this editor has been rendered.
* @param {boolean} focus - true if the editor should be focused.
*/
onceAdded() {}
onceAdded(focus) {}

/**
* Check if the editor contains something.
Expand Down
6 changes: 4 additions & 2 deletions src/display/editor/freetext.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,15 @@ class FreeTextEditor extends AnnotationEditor {
}

/** @inheritdoc */
onceAdded() {
onceAdded(focus) {
if (this.width) {
// The editor was created in using ctrl+c.
return;
}
this.enableEditMode();
this.editorDiv.focus();
if (focus) {
this.editorDiv.focus();
}
if (this._initialOptions?.isCentered) {
this.center();
}
Expand Down
6 changes: 4 additions & 2 deletions src/display/editor/highlight.js
Original file line number Diff line number Diff line change
Expand Up @@ -439,11 +439,13 @@ class HighlightEditor extends AnnotationEditor {
}

/** @inheritdoc */
onceAdded() {
onceAdded(focus) {
if (!this.annotationElementId) {
this.parent.addUndoableEditor(this);
}
this.div.focus();
if (focus) {
this.div.focus();
}
}

/** @inheritdoc */
Expand Down
6 changes: 4 additions & 2 deletions src/display/editor/stamp.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,9 +338,11 @@ class StampEditor extends AnnotationEditor {
}

/** @inheritdoc */
onceAdded() {
onceAdded(focus) {
this._isDraggable = true;
this.div.focus();
if (focus) {
this.div.focus();
}
}

/** @inheritdoc */
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 4, 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 4.
// 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.
Loading