Merge pull request #20321 from calixteman/bug1989420

[Editor] Add a fake annotation (in the annotation layer) associated with an editor in order to be able to show the comment button (bug 1989420)

src/display/annotation_layer.js

@@ -284,6 +284,24 @@ class AnnotationElement {
     );
   }
 
+  set commentText(text) {
+    const { data } = this;
+    const popup = { deleted: !text, contents: text || "" };
+    if (!this.annotationStorage.updateEditor(data.id, { popup })) {
+      this.annotationStorage.setValue(`${AnnotationEditorPrefix}${data.id}`, {
+        id: data.id,
+        annotationType: data.annotationType,
+        pageIndex: this.parent.page._pageIndex,
+        popup,
+        popupRef: data.popupRef,
+        modificationDate: new Date(),
+      });
+    }
+    if (!text) {
+      this.removePopup();
+    }
+  }
+
   removePopup() {
     (this.#popupElement?.popup || this.popup)?.remove();
     this.#popupElement = this.popup = null;
@@ -308,11 +326,9 @@ class AnnotationElement {
 
     let popup = this.#popupElement?.popup || this.popup;
     if (!popup && newPopup?.text) {
-      if (!this.parent._commentManager) {
       this._createPopup(newPopup);
       popup = this.#popupElement.popup;
     }
-    }
     if (!popup) {
       return;
     }
@@ -882,6 +898,56 @@ class AnnotationElement {
   }
 }
 
+class EditorAnnotationElement extends AnnotationElement {
+  constructor(parameters) {
+    super(parameters, { isRenderable: true, ignoreBorder: true });
+    this.editor = parameters.editor;
+  }
+
+  render() {
+    this.container.className = "editorAnnotation";
+    return this.container;
+  }
+
+  createOrUpdatePopup() {
+    const { editor } = this;
+    if (!editor.hasComment) {
+      return;
+    }
+    this._createPopup(editor.comment);
+    this.extraPopupElement.popup.renderCommentButton();
+  }
+
+  get hasCommentButton() {
+    return this.enableComment && this.editor.hasComment;
+  }
+
+  get commentButtonPosition() {
+    return this.editor.commentButtonPositionInPage;
+  }
+
+  get commentText() {
+    return this.editor.comment.text;
+  }
+
+  set commentText(text) {
+    this.editor.comment = text;
+    if (!text) {
+      this.removePopup();
+    }
+  }
+
+  get commentData() {
+    return this.editor.getData();
+  }
+
+  remove() {
+    this.container.remove();
+    this.container = null;
+    this.removePopup();
+  }
+}
+
 class LinkAnnotationElement extends AnnotationElement {
   constructor(parameters, options = null) {
     super(parameters, {
@@ -2541,7 +2607,9 @@ class PopupElement {
   }
 
   #updateCommentButtonPosition() {
-    if (this.#firstElement.extraPopupElement) {
+    if (this.#firstElement.extraPopupElement && !this.#firstElement.editor) {
+      // If there's no editor associated with the annotation then the comment
+      // button position can't be changed.
       return;
     }
     this.renderCommentButton();
@@ -2596,30 +2664,10 @@ class PopupElement {
   }
 
   set comment(text) {
-    const element = this.#firstElement;
-    const { data } = element;
     if (text === this.comment) {
       return;
     }
-    const popup = { deleted: !text, contents: text || "" };
+    this.#firstElement.commentText = this.#commentText = text;
-    if (!element.annotationStorage.updateEditor(data.id, { popup })) {
-      element.annotationStorage.setValue(
-        `${AnnotationEditorPrefix}${data.id}`,
-        {
-          id: data.id,
-          annotationType: data.annotationType,
-          pageIndex: element.parent.page._pageIndex,
-          popup,
-          popupRef: data.popupRef,
-          modificationDate: new Date(),
-        }
-      );
-    }
-
-    this.#commentText = text;
-    if (!text) {
-      element.removePopup();
-    }
   }
 
   get parentBoundingClientRect() {
@@ -3681,10 +3729,14 @@ class AnnotationLayer {
 
   #annotationCanvasMap = null;
 
+  #annotationStorage = null;
+
   #editableAnnotations = new Map();
 
   #structTreeLayer = null;
 
+  #linkService = null;
+
   constructor({
     div,
     accessibilityManager,
@@ -3694,11 +3746,15 @@ class AnnotationLayer {
     viewport,
     structTreeLayer,
     commentManager,
+    linkService,
+    annotationStorage,
   }) {
     this.div = div;
     this.#accessibilityManager = accessibilityManager;
     this.#annotationCanvasMap = annotationCanvasMap;
     this.#structTreeLayer = structTreeLayer || null;
+    this.#linkService = linkService || null;
+    this.#annotationStorage = annotationStorage || new AnnotationStorage();
     this.page = page;
     this.viewport = viewport;
     this.zIndex = 0;
@@ -3762,12 +3818,12 @@ class AnnotationLayer {
     const elementParams = {
       data: null,
       layer,
-      linkService: params.linkService,
+      linkService: this.#linkService,
       downloadManager: params.downloadManager,
       imageResourcesPath: params.imageResourcesPath || "",
       renderForms: params.renderForms !== false,
       svgFactory: new DOMSVGFactory(),
-      annotationStorage: params.annotationStorage || new AnnotationStorage(),
+      annotationStorage: this.#annotationStorage,
       enableComment: params.enableComment === true,
       enableScripting: params.enableScripting === true,
       hasJSActions: params.hasJSActions,
@@ -3832,11 +3888,11 @@ class AnnotationLayer {
    * @param {IPDFLinkService} linkService
    * @memberof AnnotationLayer
    */
-  async addLinkAnnotations(annotations, linkService) {
+  async addLinkAnnotations(annotations) {
     const elementParams = {
       data: null,
       layer: this.div,
-      linkService,
+      linkService: this.#linkService,
       svgFactory: new DOMSVGFactory(),
       parent: this,
     };
@@ -3919,6 +3975,34 @@ class AnnotationLayer {
     return this.#editableAnnotations.get(id);
   }
 
+  addFakeAnnotation(editor) {
+    const { div } = this;
+    const { id, rotation } = editor;
+    const element = new EditorAnnotationElement({
+      data: {
+        id,
+        rect: editor.getPDFRect(),
+        rotation,
+      },
+      editor,
+      layer: div,
+      parent: this,
+      enableComment: !!this._commentManager,
+      linkService: this.#linkService,
+      annotationStorage: this.#annotationStorage,
+    });
+    const htmlElement = element.render();
+    div.append(htmlElement);
+    this.#accessibilityManager?.moveElementInDOM(
+      div,
+      htmlElement,
+      htmlElement,
+      /* isRemovable = */ false
+    );
+    element.createOrUpdatePopup();
+    return element;
+  }
+
   /**
    * @private
    */

src/display/editor/annotation_editor_layer.js

@@ -170,6 +170,7 @@ class AnnotationEditorLayer {
     this.#cleanup();
     switch (mode) {
       case AnnotationEditorType.NONE:
+        this.div.classList.toggle("nonEditing", true);
         this.disableTextSelection();
         this.togglePointerEvents(false);
         this.toggleAnnotationLayerPointerEvents(true);
@@ -193,6 +194,7 @@ class AnnotationEditorLayer {
 
     this.toggleAnnotationLayerPointerEvents(false);
     const { classList } = this.div;
+    classList.toggle("nonEditing", false);
     if (mode === AnnotationEditorType.POPUP) {
       classList.toggle("commentEditing", true);
     } else {
@@ -257,6 +259,7 @@ class AnnotationEditorLayer {
     this.#isEnabling = true;
     this.div.tabIndex = 0;
     this.togglePointerEvents(true);
+    this.div.classList.toggle("nonEditing", false);
     this.#textLayerDblClickAC?.abort();
     this.#textLayerDblClickAC = null;
     const annotationElementIds = new Set();
@@ -269,13 +272,9 @@ class AnnotationEditorLayer {
       }
     }
 
-    if (!this.#annotationLayer) {
+    const annotationLayer = this.#annotationLayer;
-      this.#isEnabling = false;
+    if (annotationLayer) {
-      return;
+      for (const editable of annotationLayer.getEditableAnnotations()) {
-    }
-
-    const editables = this.#annotationLayer.getEditableAnnotations();
-    for (const editable of editables) {
         // The element must be hidden whatever its state is.
         editable.hide();
         if (this.#uiManager.isDeletedAnnotationElement(editable.data.id)) {
@@ -291,6 +290,7 @@ class AnnotationEditorLayer {
         this.addOrRebuild(editor);
         editor.enableEditing();
       }
+    }
     this.#isEnabling = false;
     this.#uiManager._eventBus.dispatch("editorsrendered", {
       source: this,
@@ -305,6 +305,7 @@ class AnnotationEditorLayer {
     this.#isDisabling = true;
     this.div.tabIndex = -1;
     this.togglePointerEvents(false);
+    this.div.classList.toggle("nonEditing", true);
     if (this.#textLayer && !this.#textLayerDblClickAC) {
       this.#textLayerDblClickAC = new AbortController();
       const signal = this.#uiManager.combinedSignal(this.#textLayerDblClickAC);
@@ -351,11 +352,15 @@ class AnnotationEditorLayer {
         { signal, capture: true }
       );
     }
+
+    const annotationLayer = this.#annotationLayer;
+    if (annotationLayer) {
       const changedAnnotations = new Map();
       const resetAnnotations = new Map();
       for (const editor of this.#allEditorsIterator) {
         editor.disableEditing();
         if (!editor.annotationElementId) {
+          editor.updateFakeAnnotationElement(annotationLayer);
           continue;
         }
         if (editor.serialize() !== null) {
@@ -368,9 +373,8 @@ class AnnotationEditorLayer {
         editor.remove();
       }
 
-    if (this.#annotationLayer) {
       // Show the annotations that were hidden in enable().
-      const editables = this.#annotationLayer.getEditableAnnotations();
+      const editables = annotationLayer.getEditableAnnotations();
       for (const editable of editables) {
         const { id } = editable.data;
         if (this.#uiManager.isDeletedAnnotationElement(id)) {
@@ -725,7 +729,7 @@ class AnnotationEditorLayer {
   /**
    * Create a new editor
    * @param {Object} data
-   * @returns {AnnotationEditor | null}
+   * @returns {Promise<AnnotationEditor | null>}
    */
   async deserialize(data) {
     return (

src/display/editor/editor.js

@@ -69,6 +69,8 @@ class AnnotationEditor {
 
   #savedDimensions = null;
 
+  #fakeAnnotation = null;
+
   #focusAC = null;
 
   #focusedResizerName = "";
@@ -382,6 +384,10 @@ class AnnotationEditor {
     } else {
       // The editor is being removed from the DOM, so we need to stop resizing.
       this.#stopResizing();
+
+      // Remove the fake annotation in the annotation layer.
+      this.#fakeAnnotation?.remove();
+      this.#fakeAnnotation = null;
     }
     this.parent = parent;
   }
@@ -1172,7 +1178,9 @@ class AnnotationEditor {
 
   addStandaloneCommentButton() {
     if (this.#commentStandaloneButton) {
+      if (this._uiManager.isEditingMode()) {
         this.#commentStandaloneButton.classList.remove("hidden");
+      }
       return;
     }
     if (!this.hasComment) {
@@ -2338,6 +2346,24 @@ class AnnotationEditor {
     this.#disabled = true;
   }
 
+  updateFakeAnnotationElement(annotationLayer) {
+    if (!this.#fakeAnnotation && !this.deleted) {
+      this.#fakeAnnotation = annotationLayer.addFakeAnnotation(this);
+      return;
+    }
+    if (this.deleted) {
+      this.#fakeAnnotation.remove();
+      this.#fakeAnnotation = null;
+      return;
+    }
+    if (this.hasEditedComment || this._hasBeenMoved || this._hasBeenResized) {
+      this.#fakeAnnotation.updateEdited({
+        rect: this.getPDFRect(),
+        popup: this.comment,
+      });
+    }
+  }
+
   /**
    * Render an annotation in the annotation layer.
    * @param {Object} annotation

src/display/editor/tools.js

@@ -2668,6 +2668,10 @@ class AnnotationEditorUIManager {
     return this.#mode;
   }
 
+  isEditingMode() {
+    return this.#mode !== AnnotationEditorType.NONE;
+  }
+
   get imageManager() {
     return shadow(this, "imageManager", new ImageManager());
   }

test/integration/comment_spec.mjs

@@ -41,6 +41,21 @@ const highlightSpan = async (page, pageIndex, text) => {
   await page.waitForSelector(getEditorSelector(0));
 };
 
+const editComment = async (page, editorSelector, comment) => {
+  const commentButtonSelector = `${editorSelector} button.comment`;
+  await waitAndClick(page, commentButtonSelector);
+
+  const textInputSelector = "#commentManagerTextInput";
+  await page.waitForSelector(textInputSelector, {
+    visible: true,
+  });
+  await page.type(textInputSelector, comment);
+  await waitAndClick(page, "#commentManagerSaveButton");
+  await page.waitForSelector("#commentManagerDialog", {
+    visible: false,
+  });
+};
+
 describe("Comment", () => {
   describe("Comment edit dialog must be visible in ltr", () => {
     let pages;
@@ -88,10 +103,10 @@ describe("Comment", () => {
           }));
           expect(dialogRect.x + dialogRect.width)
             .withContext(`In ${browserName}`)
-            .toBeLessThanOrEqual(viewport.width);
+            .toBeLessThanOrEqual(viewport.width + 1);
           expect(dialogRect.y + dialogRect.height)
             .withContext(`In ${browserName}`)
-            .toBeLessThanOrEqual(viewport.height);
+            .toBeLessThanOrEqual(viewport.height + 1);
         })
       );
     });
@@ -134,14 +149,14 @@ describe("Comment", () => {
           });
           const dialogRect = await getRect(page, "#commentManagerDialog");
           const viewport = await page.evaluate(() => ({
-            height: document.documentElement.clientHeight,
+            height: window.innerHeight,
           }));
           expect(dialogRect.x + dialogRect.width)
             .withContext(`In ${browserName}`)
-            .toBeGreaterThanOrEqual(0);
+            .toBeGreaterThanOrEqual(-1);
           expect(dialogRect.y + dialogRect.height)
             .withContext(`In ${browserName}`)
-            .toBeLessThanOrEqual(viewport.height);
+            .toBeLessThanOrEqual(viewport.height + 1);
         })
       );
     });
@@ -309,6 +324,55 @@ describe("Comment", () => {
         })
       );
     });
+
+    it("must check that the comment button is added in the annotation layer", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await switchToHighlight(page);
+
+          const rect = await getSpanRectFromText(page, 1, "Abstract");
+          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 comment = "Hello world!";
+          await editComment(page, getEditorSelector(0), comment);
+          await page.hover("#editorHighlightButton");
+          let buttonSelector =
+            ".annotationEditorLayer .annotationCommentButton";
+          await page.waitForSelector(buttonSelector, { visible: true });
+          await page.hover(buttonSelector);
+          const popupSelector = "#commentPopup";
+          await page.waitForSelector(popupSelector, {
+            visible: true,
+          });
+          let popupText = await page.evaluate(
+            selector => document.querySelector(selector).textContent,
+            `${popupSelector} .commentPopupText`
+          );
+          expect(popupText).withContext(`In ${browserName}`).toEqual(comment);
+
+          await page.hover("#editorHighlightButton");
+          await switchToHighlight(page, /* disable = */ true);
+
+          buttonSelector = ".annotationLayer .annotationCommentButton";
+          await page.waitForSelector(buttonSelector, {
+            visible: true,
+          });
+          await page.hover(buttonSelector);
+
+          await page.waitForSelector(popupSelector, {
+            visible: true,
+          });
+          popupText = await page.evaluate(
+            selector => document.querySelector(selector).textContent,
+            `${popupSelector} .commentPopupText`
+          );
+          expect(popupText).withContext(`In ${browserName}`).toEqual(comment);
+        })
+      );
+    });
   });
 
   describe("Focused element after editing", () => {

web/annotation_editor_layer_builder.css

@@ -69,6 +69,13 @@
   }
 }
 
+.page:has(.annotationEditorLayer.nonEditing)
+  .annotationLayer
+  .editorAnnotation {
+  position: absolute;
+  pointer-events: none;
+}
+
 #viewerContainer.pdfPresentationMode:fullscreen,
 .annotationEditorLayer.disabled {
   .noAltTextBadge {

web/annotation_layer_builder.js

@@ -64,13 +64,6 @@ import { PresentationModeState } from "./ui_utils.js";
  * @property {StructTreeLayerBuilder} [structTreeLayer]
  */
 
-/**
- * @typedef {Object} InjectLinkAnnotationsOptions
- * @property {Array<Object>} inferredLinks
- * @property {PageViewport} viewport
- * @property {StructTreeLayerBuilder} [structTreeLayer]
- */
-
 class AnnotationLayerBuilder {
   #annotations = null;
 
@@ -158,23 +151,19 @@ class AnnotationLayerBuilder {
     const div = (this.div = document.createElement("div"));
     div.className = "annotationLayer";
     this.#onAppend?.(div);
+    this.#initAnnotationLayer(viewport, structTreeLayer);
 
     if (annotations.length === 0) {
       this.#annotations = annotations;
-
+      setLayerDimensions(this.div, viewport);
-      this.hide(/* internal = */ true);
       return;
     }
 
-    this.#initAnnotationLayer(viewport, structTreeLayer);
-
     await this.annotationLayer.render({
       annotations,
       imageResourcesPath: this.imageResourcesPath,
       renderForms: this.renderForms,
-      linkService: this.linkService,
       downloadManager: this.downloadManager,
-      annotationStorage: this.annotationStorage,
       enableComment: this.enableComment,
       enableScripting: this.enableScripting,
       hasJSActions,
@@ -207,10 +196,12 @@ class AnnotationLayerBuilder {
       accessibilityManager: this._accessibilityManager,
       annotationCanvasMap: this._annotationCanvasMap,
       annotationEditorUIManager: this._annotationEditorUIManager,
+      annotationStorage: this.annotationStorage,
       page: this.pdfPage,
       viewport: viewport.clone({ dontFlip: true }),
       structTreeLayer,
       commentManager: this.#commentManager,
+      linkService: this.linkService,
     });
   }
 
@@ -234,15 +225,11 @@ class AnnotationLayerBuilder {
   }
 
   /**
-   * @param {InjectLinkAnnotationsOptions} options
+   * @param {Array<Object>} inferredLinks
    * @returns {Promise<void>} A promise that is resolved when the inferred links
    *   are added to the annotation layer.
    */
-  async injectLinkAnnotations({
+  async injectLinkAnnotations(inferredLinks) {
-    inferredLinks,
-    viewport,
-    structTreeLayer = null,
-  }) {
     if (this.#annotations === null) {
       throw new Error(
         "`render` method must be called before `injectLinkAnnotations`."
@@ -261,12 +248,7 @@ class AnnotationLayerBuilder {
       return;
     }
 
-    if (!this.annotationLayer) {
+    await this.annotationLayer.addLinkAnnotations(newLinks);
-      this.#initAnnotationLayer(viewport, structTreeLayer);
-      setLayerDimensions(this.div, viewport);
-    }
-
-    await this.annotationLayer.addLinkAnnotations(newLinks, this.linkService);
     // Don't show the annotation layer if it was explicitly hidden previously.
     if (!this.#externalHide) {
       this.div.hidden = false;

web/pdf_page_view.js

@@ -521,11 +521,9 @@ class PDFPageView extends BasePDFPageView {
       if (!this.annotationLayer) {
         return; // Rendering was cancelled while the textLayerPromise resolved.
       }
-      await this.annotationLayer.injectLinkAnnotations({
+      await this.annotationLayer.injectLinkAnnotations(
-        inferredLinks: Autolinker.processLinks(this),
+        Autolinker.processLinks(this)
-        viewport: this.viewport,
+      );
-        structTreeLayer: this.structTreeLayer,
-      });
     } catch (ex) {
       console.error("#injectLinkAnnotations:", ex);
       error = ex;
@@ -1120,6 +1118,10 @@ class PDFPageView extends BasePDFPageView {
       await this.#renderDrawLayer();
       this.drawLayer.setParent(canvasWrapper);
 
+      if (
+        this.annotationLayer ||
+        this.#annotationMode === AnnotationMode.DISABLE
+      ) {
         this.annotationEditorLayer ||= new AnnotationEditorLayerBuilder({
           uiManager: annotationEditorUIManager,
           pdfPage,
@@ -1134,6 +1136,7 @@ class PDFPageView extends BasePDFPageView {
           },
         });
         this.#renderAnnotationEditorLayer();
+      }
     });
 
     if (pdfPage.isPureXfa) {