Fix the z-order of ink drawings relative to the other editors

src/display/draw_layer.js

@@ -813,6 +813,22 @@ class DrawLayer {
     }
   }
 
+  /**
+   * The SVGs live in the canvas wrapper whereas the editor divs live in the
+   * annotation editor layer: both are siblings, hence the SVGs must carry the
+   * same z-index as their editor in order to be stacked with them.
+   *
+   * @param {number} id
+   * @param {number} zIndex
+   * @returns {undefined}
+   */
+  updateZIndex(id, zIndex) {
+    const root = this.#mapping.get(id);
+    if (root) {
+      root.style.zIndex = zIndex;
+    }
+  }
+
   updateParent(id, layer) {
     if (layer === this) {
       return;

src/display/editor/draw.js

@@ -124,6 +124,7 @@ class DrawingEditor extends AnnotationEditor {
         drawId,
         drawOutlines.defaultProperties
       );
+      this.parent.drawLayer.updateZIndex(drawId, this.zIndex);
     } else {
       // We create a new drawing.
       this._drawId = this.#createDrawing(drawOutlines, this.parent);
@@ -141,9 +142,29 @@ class DrawingEditor extends AnnotationEditor {
       /* isPathUpdatable = */ false,
       /* hasClip = */ false
     );
+    // The SVG lives in the canvas wrapper, which is a sibling of the annotation
+    // editor layer containing the editor divs, so it must share their z-index
+    // in order to be stacked with the other editors.
+    parent.drawLayer.updateZIndex(id, this.zIndex);
     return id;
   }
 
+  /** @inheritdoc */
+  setInBackground() {
+    super.setInBackground();
+    if (this._drawId !== null) {
+      this.parent?.drawLayer.updateZIndex(this._drawId, 0);
+    }
+  }
+
+  /** @inheritdoc */
+  setInForeground() {
+    super.setInForeground();
+    if (this._drawId !== null) {
+      this.parent?.drawLayer.updateZIndex(this._drawId, this.zIndex);
+    }
+  }
+
   static _mergeSVGProperties(p1, p2) {
     const p1Keys = new Set(Object.keys(p1));
 
@@ -348,6 +369,28 @@ class DrawingEditor extends AnnotationEditor {
     );
   }
 
+  /** @inheritdoc */
+  select() {
+    super.select();
+    // The `.selectedEditor` rule raises the editor div, but the drawing itself
+    // lives in a separate SVG, which must be raised along with it.
+    this.parent?.drawLayer.updateProperties(this._drawId, {
+      rootClass: {
+        selected: true,
+      },
+    });
+  }
+
+  /** @inheritdoc */
+  unselect() {
+    super.unselect();
+    this.parent?.drawLayer.updateProperties(this._drawId, {
+      rootClass: {
+        selected: false,
+      },
+    });
+  }
+
   _onStartDragging() {
     this.parent?.drawLayer.updateProperties(this._drawId, {
       rootClass: {
@@ -831,6 +874,12 @@ class DrawingEditor extends AnnotationEditor {
       /* isPathUpdatable = */ true,
       /* hasClip = */ false
     ));
+    // The editor isn't created until the drawing session ends, so use the
+    // z-index it will be given in order to draw on top of the existing editors.
+    parent.drawLayer.updateZIndex(
+      this._currentDrawId,
+      AnnotationEditor._zIndex
+    );
   }
 
   static _drawMove(event) {

src/display/editor/editor.js

@@ -371,6 +371,13 @@ class AnnotationEditor {
     return this._uiManager.currentLayer;
   }
 
+  /**
+   * @returns {number} the z-index used to stack this editor with the others.
+   */
+  get zIndex() {
+    return this.#zIndex;
+  }
+
   /**
    * This editor will be behind the others.
    */

test/integration/ink_editor_spec.mjs

@@ -17,6 +17,7 @@ import {
   awaitPromise,
   clearEditors,
   closePages,
+  copyToClipboard,
   countStorageEntries,
   createPromise,
   dragAndDrop,
@@ -26,15 +27,18 @@ import {
   getRect,
   getSerialized,
   isCanvasMonochrome,
+  kbBigMoveDown,
   kbRedo,
   kbSave,
   kbUndo,
   loadAndWait,
   moveEditor,
+  pasteFromClipboard,
   scrollIntoView,
   selectEditor,
   selectEditors,
   switchToEditor,
+  unselectEditor,
   waitForAnnotationModeChanged,
   waitForNoElement,
   waitForPointerUp,
@@ -44,6 +48,10 @@ import {
   waitForTimeout,
 } from "./test_utils.mjs";
 import { AnnotationEditorType } from "../../src/shared/util.js";
+import fs from "fs";
+import path from "path";
+
+const __dirname = import.meta.dirname;
 
 const selectAll = selectEditors.bind(null, "ink");
 
@@ -56,6 +64,29 @@ const commit = async page => {
 
 const switchToInk = switchToEditor.bind(null, "Ink");
 
+const switchToStamp = switchToEditor.bind(null, "Stamp");
+
+const copyImage = async (page, imagePath, selector) => {
+  const data = fs
+    .readFileSync(path.join(__dirname, imagePath))
+    .toString("base64");
+
+  await copyToClipboard(page, { "image/png": `data:image/png;base64,${data}` });
+  await pasteFromClipboard(page);
+
+  await page.waitForSelector(`${selector} canvas`);
+  await page.waitForSelector(`${selector} .altText`);
+};
+
+// The drawings are in SVGs living in the canvas wrapper, whereas the other
+// editors are divs living in the annotation editor layer. Both layers are
+// siblings, so the z-index is what decides which one is on top.
+const getZIndex = (page, selector) =>
+  page.evaluate(sel => {
+    const { zIndex } = getComputedStyle(document.querySelector(sel));
+    return zIndex === "auto" ? 0 : parseInt(zIndex, 10);
+  }, selector);
+
 const drawLine = async (page, x0, y0, x1, y1) => {
   const clickHandle = await waitForPointerUp(page);
   await page.mouse.move(x0, y0);
@@ -1407,3 +1438,64 @@ describe("Ink must be committed when the document is saved", () => {
     );
   });
 });
+
+describe("Ink stacking with a stamp", () => {
+  let pages;
+
+  beforeEach(async () => {
+    pages = await loadAndWait("empty.pdf", ".annotationEditorLayer");
+  });
+
+  afterEach(async () => {
+    await closePages(pages);
+  });
+
+  it("must draw on top of a stamp added before", async () => {
+    // Run sequentially to avoid clipboard issues.
+    for (const [browserName, page] of pages) {
+      await switchToStamp(page);
+      const stampSelector = getEditorSelector(0);
+      await copyImage(page, "../images/firefox_logo.png", stampSelector);
+      // A pasted stamp is centered, so move it away in order to draw on an
+      // empty area: the stamp would otherwise intercept the pointer events.
+      await moveEditor(page, stampSelector, 10, () => kbBigMoveDown(page));
+      await unselectEditor(page, stampSelector);
+
+      await switchToInk(page);
+      const rect = await getRect(page, ".annotationEditorLayer");
+      const x = rect.x + 100;
+      const y = rect.y + 100;
+      await drawLine(page, x, y, x + 50, y + 50);
+      await commit(page);
+      // Unselect: a selected editor is always brought to the front, which
+      // would hide the stacking we want to check here.
+      await unselectEditor(page, getEditorSelector(1));
+
+      expect(await getZIndex(page, ".canvasWrapper .draw"))
+        .withContext(`In ${browserName}`)
+        .toBeGreaterThan(await getZIndex(page, stampSelector));
+    }
+  });
+
+  it("must be under a stamp added after", async () => {
+    // Run sequentially to avoid clipboard issues.
+    for (const [browserName, page] of pages) {
+      await switchToInk(page);
+      const rect = await getRect(page, ".annotationEditorLayer");
+      const x = rect.x + 100;
+      const y = rect.y + 100;
+      await drawLine(page, x, y, x + 50, y + 50);
+      await commit(page);
+      await unselectEditor(page, getEditorSelector(0));
+
+      await switchToStamp(page);
+      const stampSelector = getEditorSelector(1);
+      await copyImage(page, "../images/firefox_logo.png", stampSelector);
+      await unselectEditor(page, stampSelector);
+
+      expect(await getZIndex(page, ".canvasWrapper .draw"))
+        .withContext(`In ${browserName}`)
+        .toBeLessThan(await getZIndex(page, stampSelector));
+    }
+  });
+});

web/draw_layer_builder.css

@@ -49,6 +49,19 @@
       position: absolute;
       mix-blend-mode: normal;
 
+      /* The SVG is purely visual: the editor div, in the annotation editor
+       * layer, is what handles the interactions. Since the SVG is stacked with
+       * the editors, it would otherwise be hit-tested before the layer and
+       * swallow the pointer events used to draw. */
+      pointer-events: none;
+
+      /* Raised along with the `.selectedEditor` div it belongs to. Scoped to
+       * `.draw` because highlight outlines use `.selected` for their own
+       * styling. */
+      &.selected {
+        z-index: 100000 !important;
+      }
+
       &[data-draw-rotation="90"] {
         transform: rotate(90deg);
       }