diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index fe01b9156..1284187bd 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -193,11 +193,16 @@ class AnnotationEditorLayer { this.toggleAnnotationLayerPointerEvents(false); const { classList } = this.div; - for (const editorType of AnnotationEditorLayer.#editorTypes.values()) { - classList.toggle( - `${editorType._type}Editing`, - mode === editorType._editorType - ); + if (mode === AnnotationEditorType.POPUP) { + classList.toggle("commentEditing", true); + } else { + classList.toggle("commentEditing", false); + for (const editorType of AnnotationEditorLayer.#editorTypes.values()) { + classList.toggle( + `${editorType._type}Editing`, + mode === editorType._editorType + ); + } } this.div.hidden = false; } diff --git a/test/integration/comment_spec.mjs b/test/integration/comment_spec.mjs new file mode 100644 index 000000000..113d7543c --- /dev/null +++ b/test/integration/comment_spec.mjs @@ -0,0 +1,134 @@ +/* Copyright 2025 Mozilla Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + closePages, + getEditorSelector, + getRect, + getSpanRectFromText, + loadAndWait, + scrollIntoView, + switchToEditor, + waitAndClick, +} from "./test_utils.mjs"; + +const switchToHighlight = switchToEditor.bind(null, "Highlight"); + +describe("Comment", () => { + describe("Comment edit dialog must be visible in ltr", () => { + let pages; + + beforeEach(async () => { + pages = await loadAndWait( + "bug1989304.pdf", + ".annotationEditorLayer", + "page-width", + null, + { enableComment: true } + ); + }); + + afterEach(async () => { + await closePages(pages); + }); + + it("must set the comment dialog in the viewport (LTR)", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + + await scrollIntoView(page, ".textLayer span:last-of-type"); + const rect = await getSpanRectFromText(page, 1, "..."); + const x = rect.x + rect.width / 2; + const y = rect.y + rect.height / 2; + // Here and elsewhere, we add a small delay between press and release + // to make sure that a pointerup event is triggered after + // selectionchange. + // It works with a value of 1ms, but we use 100ms to be sure. + await page.mouse.click(x, y, { count: 2, delay: 100 }); + await page.waitForSelector(getEditorSelector(0)); + + const commentButtonSelector = `${getEditorSelector(0)} button.comment`; + await waitAndClick(page, commentButtonSelector); + + await page.waitForSelector("#commentManagerDialog", { + visible: true, + }); + const dialogRect = await getRect(page, "#commentManagerDialog"); + const viewport = await page.evaluate(() => ({ + width: document.documentElement.clientWidth, + height: document.documentElement.clientHeight, + })); + expect(dialogRect.x + dialogRect.width) + .withContext(`In ${browserName}`) + .toBeLessThanOrEqual(viewport.width); + expect(dialogRect.y + dialogRect.height) + .withContext(`In ${browserName}`) + .toBeLessThanOrEqual(viewport.height); + }) + ); + }); + }); + + describe("Comment edit dialog must be visible in rtl", () => { + let pages; + + beforeEach(async () => { + pages = await loadAndWait( + "bug1989304.pdf", + ".annotationEditorLayer", + "page-width", + null, + { enableComment: true, localeProperties: "ar" } + ); + }); + + afterEach(async () => { + await closePages(pages); + }); + + it("must set the comment dialog in the viewport (RTL)", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + + await scrollIntoView(page, ".textLayer span:nth-of-type(4)"); + const rect = await getSpanRectFromText(page, 1, "World"); + const x = rect.x + rect.width / 2; + const y = rect.y + rect.height / 2; + await page.mouse.click(x, y, { count: 2, delay: 100 }); + await page.waitForSelector(getEditorSelector(0)); + + const commentButtonSelector = `${getEditorSelector(0)} button.comment`; + await waitAndClick(page, commentButtonSelector); + + await page.waitForSelector("#commentManagerDialog", { + visible: true, + }); + const dialogRect = await getRect(page, "#commentManagerDialog"); + const viewport = await page.evaluate(() => ({ + height: document.documentElement.clientHeight, + })); + expect(dialogRect.x + dialogRect.width) + .withContext(`In ${browserName}`) + .toBeGreaterThanOrEqual(0); + expect(dialogRect.y + dialogRect.height) + .withContext(`In ${browserName}`) + .toBeLessThanOrEqual(viewport.height); + }) + ); + }); + }); +}); diff --git a/test/integration/jasmine-boot.js b/test/integration/jasmine-boot.js index 72a8412af..51590003c 100644 --- a/test/integration/jasmine-boot.js +++ b/test/integration/jasmine-boot.js @@ -30,6 +30,7 @@ async function runTests(results) { "annotation_spec.mjs", "autolinker_spec.mjs", "caret_browsing_spec.mjs", + "comment_spec.mjs", "copy_paste_spec.mjs", "document_properties_spec.mjs", "find_spec.mjs", diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore index 781516bde..609bbd87a 100644 --- a/test/pdfs/.gitignore +++ b/test/pdfs/.gitignore @@ -744,3 +744,4 @@ !bug1980958.pdf !tracemonkey_annotation_on_page_8.pdf !issue20232.pdf +!bug1989304.pdf diff --git a/test/pdfs/bug1989304.pdf b/test/pdfs/bug1989304.pdf new file mode 100755 index 000000000..c4aa51f62 Binary files /dev/null and b/test/pdfs/bug1989304.pdf differ diff --git a/web/app.js b/web/app.js index d3f752bd1..317994e2e 100644 --- a/web/app.js +++ b/web/app.js @@ -368,6 +368,7 @@ const PDFViewerApplication = { docBaseUrl: x => x, enableAltText: x => x === "true", enableAutoLinking: x => x === "true", + enableComment: x => x === "true", enableFakeMLManager: x => x === "true", enableGuessAltText: x => x === "true", enablePermissions: x => x === "true", @@ -380,6 +381,7 @@ const PDFViewerApplication = { forcePageColors: x => x === "true", pageColorsBackground: x => x, pageColorsForeground: x => x, + localeProperties: x => ({ lang: x }), }); } diff --git a/web/comment_manager.css b/web/comment_manager.css index 8dffbc714..b8323ba47 100644 --- a/web/comment_manager.css +++ b/web/comment_manager.css @@ -20,7 +20,8 @@ min-width: 200px; position: absolute; padding: 8px 16px 16px; - margin: 0; + margin-left: 0; + margin-top: 0; box-sizing: border-box; border-radius: 8px; diff --git a/web/comment_manager.js b/web/comment_manager.js index f0683eaf0..818b9c05b 100644 --- a/web/comment_manager.js +++ b/web/comment_manager.js @@ -46,7 +46,7 @@ class CommentManager { dateStyle: "long", }); this.dialogElement = commentDialog.dialog; - this.#dialog = new CommentDialog(commentDialog, overlayManager); + this.#dialog = new CommentDialog(commentDialog, overlayManager, ltr); this.#popup = new CommentPopup(dateFormat, ltr, this.dialogElement); this.#sidebar = new CommentSidebar( sidebar, @@ -572,15 +572,19 @@ class CommentDialog { #dialogY = 0; + #isLTR; + constructor( { dialog, toolbar, title, textInput, cancelButton, saveButton }, - overlayManager + overlayManager, + ltr ) { this.#dialog = dialog; this.#textInput = textInput; this.#overlayManager = overlayManager; this.#saveButton = saveButton; this.#title = title; + this.#isLTR = ltr; const finishBound = this.#finish.bind(this); dialog.addEventListener("close", finishBound); @@ -682,7 +686,38 @@ class CommentDialog { } this.#uiManager?.removeEditListeners(); this.#saveButton.disabled = true; - + const parentDimensions = options?.parentDimensions; + if (editor.hasDefaultPopupPosition()) { + const { dialogWidth, dialogHeight } = this._dialogDimensions; + if (parentDimensions) { + if ( + this.#isLTR && + posX + dialogWidth > + Math.min( + parentDimensions.x + parentDimensions.width, + window.innerWidth + ) + ) { + const buttonWidth = this.#editor.commentButtonWidth; + posX -= dialogWidth - buttonWidth * parentDimensions.width; + } else if (!this.#isLTR) { + const buttonWidth = + this.#editor.commentButtonWidth * parentDimensions.width; + if (posX - dialogWidth < Math.max(0, parentDimensions.x)) { + posX = Math.max(0, posX); + } else { + posX -= dialogWidth - buttonWidth; + } + } + } + const height = Math.max(dialogHeight, options?.height || 0); + if (posY + height > window.innerHeight) { + posY = window.innerHeight - height; + } + if (posY < 0) { + posY = 0; + } + } this.#setPosition(posX, posY); await this.#overlayManager.open(this.#dialog); @@ -694,14 +729,17 @@ class CommentDialog { this.#finish(); } - get _dialogWidth() { + get _dialogDimensions() { const dialog = this.#dialog; const { style } = dialog; style.opacity = "0"; style.display = "block"; - const width = dialog.getBoundingClientRect().width; + const { width, height } = dialog.getBoundingClientRect(); style.opacity = style.display = ""; - return shadow(this, "_dialogWidth", width); + return shadow(this, "_dialogDimensions", { + dialogWidth: width, + dialogHeight: height, + }); } #setPosition(x, y) { @@ -1021,7 +1059,7 @@ class CommentPopup { } } - #setPosition(x, y, correctPosition = true) { + #setPosition(x, y, correctPosition) { if (!correctPosition) { this.#editor.commentPopupPosition = [x, y]; } else {