From 2acada9dcf1bf7e775f4e69848ff87c0ac2adc39 Mon Sep 17 00:00:00 2001 From: Faraz Mughal Date: Mon, 20 Jul 2026 23:35:15 +0100 Subject: [PATCH] Fix the z-order of ink drawings relative to the other editors --- src/display/draw_layer.js | 16 +++++ src/display/editor/draw.js | 49 +++++++++++++++ src/display/editor/editor.js | 7 +++ test/integration/ink_editor_spec.mjs | 92 ++++++++++++++++++++++++++++ web/draw_layer_builder.css | 13 ++++ 5 files changed, 177 insertions(+) diff --git a/src/display/draw_layer.js b/src/display/draw_layer.js index 3278900ae7e38..7dbe5f407694f 100644 --- a/src/display/draw_layer.js +++ b/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; diff --git a/src/display/editor/draw.js b/src/display/editor/draw.js index b2125f173e954..42fdafeec43a4 100644 --- a/src/display/editor/draw.js +++ b/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) { diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index c18c4b8c312d7..eebce0e0b9f9f 100644 --- a/src/display/editor/editor.js +++ b/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. */ diff --git a/test/integration/ink_editor_spec.mjs b/test/integration/ink_editor_spec.mjs index c33af9504d790..bae64dfcd0470 100644 --- a/test/integration/ink_editor_spec.mjs +++ b/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)); + } + }); +}); diff --git a/web/draw_layer_builder.css b/web/draw_layer_builder.css index 5ea206ced7657..f8792ceb02d25 100644 --- a/web/draw_layer_builder.css +++ b/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); }