Skip to content

Commit 8b4f904

Browse files
committed
Add support for dismissing comment popups with click outside
This solves [bug 1989406](https://bugzilla.mozilla.org/show_bug.cgi?id=1989406). (“The user should be able to dismiss the in-content message displayed by clicking somewhere else in the PDF”) There’s a good gif there that shows the problematic behavior. In the thread, there are also mentions of 2 similar but slightly separate problems: * clicking on another highlight should also dismiss * the mention that hitting the escape key does not dismiss I found the last point, the escape key, to work already (first test case here). But this PR solves the main bug (second test case) and the adjacent one (third test case). It works by using the existing `unselectAll` handling.
1 parent d859d78 commit 8b4f904

3 files changed

Lines changed: 92 additions & 0 deletions

File tree

src/display/editor/annotation_editor_layer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,7 @@ class AnnotationEditorLayer {
901901
const currentMode = this.#uiManager.getMode();
902902
if (
903903
currentMode === AnnotationEditorType.STAMP ||
904+
currentMode === AnnotationEditorType.POPUP ||
904905
currentMode === AnnotationEditorType.SIGNATURE
905906
) {
906907
this.#uiManager.unselectAll();

src/display/editor/tools.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2393,6 +2393,7 @@ class AnnotationEditorUIManager {
23932393
ed.unselect();
23942394
}
23952395
}
2396+
this.#commentManager?.destroyPopup();
23962397
this.#selectedEditors.clear();
23972398

23982399
this.#selectedEditors.add(editor);
@@ -2583,6 +2584,8 @@ class AnnotationEditorUIManager {
25832584
return;
25842585
}
25852586

2587+
this.#commentManager?.destroyPopup();
2588+
25862589
if (!this.hasSelection) {
25872590
return;
25882591
}

test/integration/comment_spec.mjs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
dragAndDrop,
2222
getEditorSelector,
2323
getRect,
24+
getSpanRectFromText,
2425
highlightSpan,
2526
kbModifierDown,
2627
kbModifierUp,
@@ -1176,4 +1177,91 @@ describe("Comment", () => {
11761177
);
11771178
});
11781179
});
1180+
1181+
describe("Must close comment popups (bug 1989406)", () => {
1182+
let pages;
1183+
1184+
beforeEach(async () => {
1185+
pages = await loadAndWait(
1186+
"tracemonkey.pdf",
1187+
".annotationEditorLayer",
1188+
"page-fit",
1189+
null,
1190+
{ enableComment: true }
1191+
);
1192+
});
1193+
1194+
afterEach(async () => {
1195+
await closePages(pages);
1196+
});
1197+
1198+
it("must close a comment popup on escape", async () => {
1199+
await Promise.all(
1200+
pages.map(async ([browserName, page]) => {
1201+
await switchToHighlight(page);
1202+
await highlightSpan(page, 1, "Abstract");
1203+
await editComment(page, getEditorSelector(0), "hi");
1204+
const rect = await getSpanRectFromText(page, 1, "Introduction");
1205+
1206+
// Unfocus.
1207+
await page.mouse.click(rect.x, rect.y);
1208+
1209+
await waitAndClick(page, ".annotationCommentButton");
1210+
1211+
await page.waitForSelector("#commentPopup", { visible: true });
1212+
1213+
await page.keyboard.press("Escape");
1214+
1215+
await page.waitForSelector("#commentPopup", { hidden: true });
1216+
})
1217+
);
1218+
});
1219+
1220+
it("must close a comment popup on click outside", async () => {
1221+
await Promise.all(
1222+
pages.map(async ([browserName, page]) => {
1223+
await switchToHighlight(page);
1224+
await highlightSpan(page, 1, "Abstract");
1225+
await editComment(page, getEditorSelector(0), "hi");
1226+
const rect = await getSpanRectFromText(page, 1, "Introduction");
1227+
1228+
// Unfocus.
1229+
await page.mouse.click(rect.x, rect.y);
1230+
1231+
await waitAndClick(page, ".annotationCommentButton");
1232+
1233+
await page.waitForSelector("#commentPopup", { visible: true });
1234+
1235+
// Click outside the popup.
1236+
await page.mouse.click(rect.x, rect.y);
1237+
1238+
await page.waitForSelector("#commentPopup", { hidden: true });
1239+
})
1240+
);
1241+
});
1242+
1243+
it("must close a comment popup on click on other highlight", async () => {
1244+
await Promise.all(
1245+
pages.map(async ([browserName, page]) => {
1246+
await switchToHighlight(page);
1247+
1248+
await highlightSpan(page, 1, "Abstract");
1249+
await editComment(page, getEditorSelector(0), "hello");
1250+
1251+
await highlightSpan(page, 1, "Introduction");
1252+
await editComment(page, getEditorSelector(1), "world");
1253+
1254+
// Open "Abstract" comment popup.
1255+
await waitAndClick(page, ".annotationCommentButton");
1256+
1257+
await page.waitForSelector("#commentPopup", { visible: true });
1258+
1259+
// Click on "Introduction" highlight.
1260+
await waitAndClick(page, getEditorSelector(1));
1261+
1262+
await page.waitForSelector("#commentPopup", { hidden: true });
1263+
})
1264+
);
1265+
});
1266+
});
11791267
});

0 commit comments

Comments
 (0)