[Editor] Unselect correctly removed editors

- After undoing a deletion of several editors, they appeared to be selected (they had a red border)
when in fact they were not, consequently, this patch aims to remove the selectedEditor class when
an editor is removed;
- Add a test with some ink editors.

src/display/editor/ink.js

@@ -416,6 +416,9 @@ class InkEditor extends AnnotationEditor {
     // When commiting, the position of this editor is changed, hence we must
     // move it to the right position in the DOM.
     this.parent.moveDivInDOM(this);
+    // After the div has been moved in the DOM, the focus may have been stolen
+    // by document.body, hence we just keep it here.
+    this.div.focus();
   }
 
   /** @inheritdoc */

src/display/editor/tools.js

@@ -709,9 +709,7 @@ class AnnotationEditorUIManager {
    */
   removeEditor(editor) {
     this.#allEditors.delete(editor.id);
-    if (this.hasSelection) {
-      this.#selectedEditors.delete(editor);
-    }
+    this.unselect(editor);
   }
 
   /**
@@ -962,6 +960,15 @@ class AnnotationEditorUIManager {
    * Unselect all the selected editors.
    */
   unselectAll() {
+    if (this.#activeEditor) {
+      // An editor is being edited so just commit it.
+      this.#activeEditor.commitOrRemove();
+      return;
+    }
+
+    if (this.#selectEditors.size === 0) {
+      return;
+    }
     for (const editor of this.#selectedEditors) {
       editor.unselect();
     }

test/integration-boot.js

@@ -31,6 +31,7 @@ async function runTests(results) {
       "accessibility_spec.js",
       "find_spec.js",
       "freetext_editor_spec.js",
+      "ink_editor_spec.js",
     ],
   });
 

test/integration/freetext_editor_spec.js

@@ -13,9 +13,12 @@
  * limitations under the License.
  */
 
-const { closePages, loadAndWait } = require("./test_utils.js");
-
-const editorPrefix = "#pdfjs_internal_editor_";
+const {
+  closePages,
+  editorPrefix,
+  getSelectedEditors,
+  loadAndWait,
+} = require("./test_utils.js");
 
 describe("Editor", () => {
   describe("FreeText", () => {
@@ -264,18 +267,6 @@ describe("Editor", () => {
       await closePages(pages);
     });
 
-    function getSelected(page) {
-      return page.evaluate(prefix => {
-        const elements = document.querySelectorAll(".selectedEditor");
-        const results = [];
-        for (const element of elements) {
-          results.push(parseInt(element.id.slice(prefix.length)));
-        }
-        results.sort();
-        return results;
-      }, editorPrefix.slice(1));
-    }
-
     it("must select/unselect several editors and check copy, paste and delete operations", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
@@ -322,27 +313,27 @@ describe("Editor", () => {
           await page.keyboard.press("a");
           await page.keyboard.up("Control");
 
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([0, 1, 2, 3]);
 
           await page.keyboard.down("Control");
           await page.mouse.click(editorCenters[1].x, editorCenters[1].y);
 
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([0, 2, 3]);
 
           await page.mouse.click(editorCenters[2].x, editorCenters[2].y);
 
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([0, 3]);
 
           await page.mouse.click(editorCenters[1].x, editorCenters[1].y);
           await page.keyboard.up("Control");
 
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([0, 1, 3]);
 
@@ -355,25 +346,25 @@ describe("Editor", () => {
           await page.keyboard.up("Control");
 
           // 0,1,3 are unselected and new pasted editors are selected.
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([4, 5, 6]);
 
           // No ctrl here, hence all are unselected and 2 is selected.
           await page.mouse.click(editorCenters[2].x, editorCenters[2].y);
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([2]);
 
           await page.mouse.click(editorCenters[1].x, editorCenters[1].y);
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([1]);
 
           await page.keyboard.down("Control");
 
           await page.mouse.click(editorCenters[3].x, editorCenters[3].y);
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([1, 3]);
 
@@ -386,25 +377,25 @@ describe("Editor", () => {
           await page.keyboard.press("a");
           await page.keyboard.up("Control");
 
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([0, 2, 4, 5, 6]);
 
           // Create an empty editor.
           await page.mouse.click(rect.x + 700, rect.y + 100);
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([7]);
 
           // Set the focus to 2 and check that only 2 is selected.
           await page.mouse.click(editorCenters[2].x, editorCenters[2].y);
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([2]);
 
           // Create an empty editor.
           await page.mouse.click(rect.x + 700, rect.y + 100);
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([8]);
           // Dismiss it.
@@ -417,7 +408,7 @@ describe("Editor", () => {
 
           // Check that all the editors are correctly selected (and the focus
           // didn't move to the body when the empty editor was removed).
-          expect(await getSelected(page))
+          expect(await getSelectedEditors(page))
             .withContext(`In ${browserName}`)
             .toEqual([0, 2, 4, 5, 6]);
         })

test/integration/ink_editor_spec.js

@@ -0,0 +1,78 @@
+/* Copyright 2022 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.
+ */
+
+const {
+  closePages,
+  getSelectedEditors,
+  loadAndWait,
+} = require("./test_utils.js");
+
+describe("Editor", () => {
+  describe("Ink", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must draw, undo a deletion and check that the editors are not selected", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#editorInk");
+
+          const rect = await page.$eval(".annotationEditorLayer", el => {
+            // With Chrome something is wrong when serializing a DomRect,
+            // hence we extract the values and just return them.
+            const { x, y } = el.getBoundingClientRect();
+            return { x, y };
+          });
+
+          for (let i = 0; i < 3; i++) {
+            const x = rect.x + 100 + i * 100;
+            const y = rect.y + 100 + i * 100;
+            await page.mouse.move(x, y);
+            await page.mouse.down();
+            await page.mouse.move(x + 50, y + 50);
+            await page.mouse.up();
+
+            await page.keyboard.press("Escape");
+          }
+
+          await page.keyboard.down("Control");
+          await page.keyboard.press("a");
+          await page.keyboard.up("Control");
+
+          expect(await getSelectedEditors(page))
+            .withContext(`In ${browserName}`)
+            .toEqual([0, 2, 3]);
+
+          await page.keyboard.press("Backspace");
+
+          await page.keyboard.down("Control");
+          await page.keyboard.press("z");
+          await page.keyboard.up("Control");
+
+          expect(await getSelectedEditors(page))
+            .withContext(`In ${browserName}`)
+            .toEqual([]);
+        })
+      );
+    });
+  });
+});

test/integration/test_utils.js

@@ -73,3 +73,19 @@ function getComputedStyleSelector(id) {
   return `getComputedStyle(${getQuerySelector(id)})`;
 }
 exports.getComputedStyleSelector = getComputedStyleSelector;
+
+const editorPrefix = "#pdfjs_internal_editor_";
+exports.editorPrefix = editorPrefix;
+
+function getSelectedEditors(page) {
+  return page.evaluate(pref => {
+    const elements = document.querySelectorAll(".selectedEditor");
+    const results = [];
+    for (const element of elements) {
+      results.push(parseInt(element.id.slice(pref.length)));
+    }
+    results.sort();
+    return results;
+  }, editorPrefix.slice(1));
+}
+exports.getSelectedEditors = getSelectedEditors;