Merge pull request #17790 from calixteman/bug1881746

[Editor] Make the text layer focusable before the editors (bug 1881746)

src/display/editor/annotation_editor_layer.js

@@ -237,6 +237,7 @@ class AnnotationEditorLayer {
    * editor creation.
    */
   enable() {
+    this.div.tabIndex = 0;
     this.togglePointerEvents(true);
     const annotationElementIds = new Set();
     for (const editor of this.#editors.values()) {
@@ -274,6 +275,7 @@ class AnnotationEditorLayer {
    */
   disable() {
     this.#isDisabling = true;
+    this.div.tabIndex = -1;
     this.togglePointerEvents(false);
     const hiddenAnnotationIds = new Set();
     for (const editor of this.#editors.values()) {
@@ -333,6 +335,7 @@ class AnnotationEditorLayer {
   }
 
   enableTextSelection() {
+    this.div.tabIndex = -1;
     if (this.#textLayer?.div && !this.#boundTextLayerPointerDown) {
       this.#boundTextLayerPointerDown = this.#textLayerPointerDown.bind(this);
       this.#textLayer.div.addEventListener(
@@ -344,6 +347,7 @@ class AnnotationEditorLayer {
   }
 
   disableTextSelection() {
+    this.div.tabIndex = 0;
     if (this.#textLayer?.div && this.#boundTextLayerPointerDown) {
       this.#textLayer.div.removeEventListener(
         "pointerdown",

src/display/editor/editor.js

@@ -44,6 +44,8 @@ class AnnotationEditor {
 
   #altText = null;
 
+  #disabled = false;
+
   #keepAspectRatio = false;
 
   #resizersDiv = null;
@@ -1002,7 +1004,7 @@ class AnnotationEditor {
     this.div.setAttribute("data-editor-rotation", (360 - this.rotation) % 360);
     this.div.className = this.name;
     this.div.setAttribute("id", this.id);
-    this.div.setAttribute("tabIndex", 0);
+    this.div.tabIndex = this.#disabled ? -1 : 0;
     if (!this._isVisible) {
       this.div.classList.add("hidden");
     }
@@ -1680,6 +1682,20 @@ class AnnotationEditor {
     this.div.classList.toggle("hidden", !visible);
     this._isVisible = visible;
   }
+
+  enable() {
+    if (this.div) {
+      this.div.tabIndex = 0;
+    }
+    this.#disabled = false;
+  }
+
+  disable() {
+    if (this.div) {
+      this.div.tabIndex = -1;
+    }
+    this.#disabled = true;
+  }
 }
 
 // This class is used to fake an editor which has been deleted.

src/display/editor/tools.js

@@ -1596,6 +1596,9 @@ class AnnotationEditorUIManager {
       for (const layer of this.#allLayers.values()) {
         layer.enable();
       }
+      for (const editor of this.#allEditors.values()) {
+        editor.enable();
+      }
     }
   }
 
@@ -1609,6 +1612,9 @@ class AnnotationEditorUIManager {
       for (const layer of this.#allLayers.values()) {
         layer.disable();
       }
+      for (const editor of this.#allEditors.values()) {
+        editor.disable();
+      }
     }
   }
 

test/integration/freetext_editor_spec.mjs

@@ -18,6 +18,7 @@ import {
   closePages,
   createPromise,
   dragAndDropAnnotation,
+  firstPageOnTop,
   getEditors,
   getEditorSelector,
   getFirstSerialized,
@@ -39,6 +40,7 @@ import {
   kbUndo,
   loadAndWait,
   scrollIntoView,
+  waitForAnnotationEditorLayer,
   waitForEvent,
   waitForSelectedEditor,
   waitForSerialized,
@@ -923,6 +925,7 @@ describe("FreeText Editor", () => {
           let currentId = 0;
 
           for (let step = 0; step < 3; step++) {
+            await firstPageOnTop(page);
             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.
@@ -931,8 +934,8 @@ describe("FreeText Editor", () => {
             });
 
             const data = `Hello ${step}`;
-            const x = rect.x + 0.1 * rect.width;
-            const y = rect.y + 0.1 * rect.height;
+            const x = Math.max(rect.x + 0.1 * rect.width, 10);
+            const y = Math.max(rect.y + 0.1 * rect.height, 10);
             await page.mouse.click(x, y);
             await page.waitForSelector(getEditorSelector(currentId), {
               visible: true,
@@ -945,9 +948,12 @@ describe("FreeText Editor", () => {
               `${getEditorSelector(currentId)} .overlay.enabled`
             );
 
+            const promise = await waitForAnnotationEditorLayer(page);
             await page.evaluate(() => {
               document.getElementById("pageRotateCw").click();
             });
+            await awaitPromise(promise);
+
             currentId += 1;
             await page.waitForSelector(
               ".page[data-page-number='1'] .canvasWrapper",

test/integration/highlight_editor_spec.mjs

@@ -22,6 +22,8 @@ import {
   getSerialized,
   kbBigMoveLeft,
   kbBigMoveUp,
+  kbFocusNext,
+  kbFocusPrevious,
   kbSelectAll,
   loadAndWait,
   scrollIntoView,
@@ -1556,4 +1558,52 @@ describe("Highlight Editor", () => {
       );
     });
   });
+
+  describe("Text layer must have the focus before highlights", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check the focus order", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#editorHighlight");
+          await page.waitForSelector(".annotationEditorLayer.highlightEditing");
+
+          let rect = await getSpanRectFromText(page, 1, "Abstract");
+          let x = rect.x + rect.width / 2;
+          let y = rect.y + rect.height / 2;
+          await page.mouse.click(x, y, { count: 2, delay: 100 });
+          await page.waitForSelector(getEditorSelector(0));
+
+          rect = await getSpanRectFromText(page, 1, "Languages");
+          x = rect.x + rect.width / 2;
+          y = rect.y + rect.height / 2;
+          await page.mouse.click(x, y, { count: 2, delay: 100 });
+          await page.waitForSelector(getEditorSelector(1));
+          await page.focus(getEditorSelector(1));
+
+          await kbFocusPrevious(page);
+          await page.waitForFunction(
+            sel => document.querySelector(sel) === document.activeElement,
+            {},
+            `.page[data-page-number="1"] > .textLayer`
+          );
+
+          await kbFocusNext(page);
+          await page.waitForFunction(
+            sel => document.querySelector(sel) === document.activeElement,
+            {},
+            getEditorSelector(1)
+          );
+        })
+      );
+    });
+  });
 });

test/integration/test_utils.mjs

@@ -318,9 +318,12 @@ async function scrollIntoView(page, selector) {
   const handle = await page.evaluateHandle(
     sel => [
       new Promise(resolve => {
-        document
-          .getElementById("viewerContainer")
-          .addEventListener("scrollend", resolve, { once: true });
+        const container = document.getElementById("viewerContainer");
+        if (container.scrollHeight <= container.clientHeight) {
+          resolve();
+          return;
+        }
+        container.addEventListener("scrollend", resolve, { once: true });
         const element = document.querySelector(sel);
         element.scrollIntoView({ behavior: "instant", block: "start" });
       }),
@@ -330,6 +333,21 @@ async function scrollIntoView(page, selector) {
   return awaitPromise(handle);
 }
 
+async function firstPageOnTop(page) {
+  const handle = await page.evaluateHandle(() => [
+    new Promise(resolve => {
+      const container = document.getElementById("viewerContainer");
+      if (container.scrollTop === 0 && container.scrollLeft === 0) {
+        resolve();
+        return;
+      }
+      container.addEventListener("scrollend", resolve, { once: true });
+      container.scrollTo(0, 0);
+    }),
+  ]);
+  return awaitPromise(handle);
+}
+
 async function hover(page, selector) {
   const rect = await page.$eval(selector, el => {
     const { x, y, width, height } = el.getBoundingClientRect();
@@ -461,12 +479,31 @@ async function kbDeleteLastWord(page) {
   }
 }
 
+async function kbFocusNext(page) {
+  const handle = await createPromise(page, resolve => {
+    window.addEventListener("focusin", resolve, { once: true });
+  });
+  await page.keyboard.press("Tab");
+  await awaitPromise(handle);
+}
+
+async function kbFocusPrevious(page) {
+  const handle = await createPromise(page, resolve => {
+    window.addEventListener("focusin", resolve, { once: true });
+  });
+  await page.keyboard.down("Shift");
+  await page.keyboard.press("Tab");
+  await page.keyboard.up("Shift");
+  await awaitPromise(handle);
+}
+
 export {
   awaitPromise,
   clearInput,
   closePages,
   createPromise,
   dragAndDropAnnotation,
+  firstPageOnTop,
   getAnnotationStorage,
   getComputedStyleSelector,
   getEditorDimensions,
@@ -484,6 +521,8 @@ export {
   kbBigMoveUp,
   kbCopy,
   kbDeleteLastWord,
+  kbFocusNext,
+  kbFocusPrevious,
   kbGoToBegin,
   kbGoToEnd,
   kbModifierDown,

web/annotation_editor_layer_builder.css

@@ -109,7 +109,6 @@
   font-size: calc(100px * var(--scale-factor));
   transform-origin: 0 0;
   cursor: auto;
-  z-index: 4;
 }
 
 .annotationEditorLayer.waiting {

web/annotation_editor_layer_builder.js

@@ -30,13 +30,13 @@ import { GenericL10n } from "web-null_l10n";
 /**
  * @typedef {Object} AnnotationEditorLayerBuilderOptions
  * @property {AnnotationEditorUIManager} [uiManager]
- * @property {HTMLDivElement} pageDiv
  * @property {PDFPageProxy} pdfPage
  * @property {IL10n} [l10n]
  * @property {TextAccessibilityManager} [accessibilityManager]
  * @property {AnnotationLayer} [annotationLayer]
  * @property {TextLayer} [textLayer]
  * @property {DrawLayer} [drawLayer]
+ * @property {function} [onAppend]
  */
 
 class AnnotationEditorLayerBuilder {
@@ -44,6 +44,8 @@ class AnnotationEditorLayerBuilder {
 
   #drawLayer = null;
 
+  #onAppend = null;
+
   #textLayer = null;
 
   #uiManager;
@@ -52,7 +54,6 @@ class AnnotationEditorLayerBuilder {
    * @param {AnnotationEditorLayerBuilderOptions} options
    */
   constructor(options) {
-    this.pageDiv = options.pageDiv;
     this.pdfPage = options.pdfPage;
     this.accessibilityManager = options.accessibilityManager;
     this.l10n = options.l10n;
@@ -66,6 +67,7 @@ class AnnotationEditorLayerBuilder {
     this.#annotationLayer = options.annotationLayer || null;
     this.#textLayer = options.textLayer || null;
     this.#drawLayer = options.drawLayer || null;
+    this.#onAppend = options.onAppend || null;
   }
 
   /**
@@ -91,10 +93,9 @@ class AnnotationEditorLayerBuilder {
     // Create an AnnotationEditor layer div
     const div = (this.div = document.createElement("div"));
     div.className = "annotationEditorLayer";
-    div.tabIndex = 0;
     div.hidden = true;
     div.dir = this.#uiManager.direction;
-    this.pageDiv.append(div);
+    this.#onAppend?.(div);
 
     this.annotationEditorLayer = new AnnotationEditorLayer({
       uiManager: this.#uiManager,
@@ -125,7 +126,6 @@ class AnnotationEditorLayerBuilder {
     if (!this.div) {
       return;
     }
-    this.pageDiv = null;
     this.annotationEditorLayer.destroy();
     this.div.remove();
   }

web/annotation_layer_builder.css

@@ -76,7 +76,6 @@
   left: 0;
   pointer-events: none;
   transform-origin: 0 0;
-  z-index: 3;
 
   &[data-main-rotation="90"] .norotate {
     transform: rotate(270deg) translateX(-100%);

web/annotation_layer_builder.js

@@ -28,7 +28,6 @@ import { PresentationModeState } from "./ui_utils.js";
 
 /**
  * @typedef {Object} AnnotationLayerBuilderOptions
- * @property {HTMLDivElement} pageDiv
  * @property {PDFPageProxy} pdfPage
  * @property {AnnotationStorage} [annotationStorage]
  * @property {string} [imageResourcesPath] - Path for image resources, mainly
@@ -42,16 +41,18 @@ import { PresentationModeState } from "./ui_utils.js";
  *   [fieldObjectsPromise]
  * @property {Map<string, HTMLCanvasElement>} [annotationCanvasMap]
  * @property {TextAccessibilityManager} [accessibilityManager]
+ * @property {function} [onAppend]
  */
 
 class AnnotationLayerBuilder {
+  #onAppend = null;
+
   #onPresentationModeChanged = null;
 
   /**
    * @param {AnnotationLayerBuilderOptions} options
    */
   constructor({
-    pageDiv,
     pdfPage,
     linkService,
     downloadManager,
@@ -63,8 +64,8 @@ class AnnotationLayerBuilder {
     fieldObjectsPromise = null,
     annotationCanvasMap = null,
     accessibilityManager = null,
+    onAppend = null,
   }) {
-    this.pageDiv = pageDiv;
     this.pdfPage = pdfPage;
     this.linkService = linkService;
     this.downloadManager = downloadManager;
@@ -76,6 +77,7 @@ class AnnotationLayerBuilder {
     this._fieldObjectsPromise = fieldObjectsPromise || Promise.resolve(null);
     this._annotationCanvasMap = annotationCanvasMap;
     this._accessibilityManager = accessibilityManager;
+    this.#onAppend = onAppend;
 
     this.annotationLayer = null;
     this.div = null;
@@ -115,7 +117,7 @@ class AnnotationLayerBuilder {
     // if there is at least one annotation.
     const div = (this.div = document.createElement("div"));
     div.className = "annotationLayer";
-    this.pageDiv.append(div);
+    this.#onAppend?.(div);
 
     if (annotations.length === 0) {
       this.hide();

web/pdf_page_view.js

@@ -99,6 +99,14 @@ const DEFAULT_LAYER_PROPERTIES =
         },
       };
 
+const LAYERS_ORDER = new Map([
+  ["canvasWrapper", 0],
+  ["textLayer", 1],
+  ["annotationLayer", 2],
+  ["annotationEditorLayer", 3],
+  ["xfaLayer", 2],
+]);
+
 /**
  * @implements {IRenderableView}
  */
@@ -127,6 +135,8 @@ class PDFPageView {
 
   #viewportMap = new WeakMap();
 
+  #layers = [null, null, null, null];
+
   /**
    * @param {PDFPageViewOptions} options
    */
@@ -223,6 +233,19 @@ class PDFPageView {
     }
   }
 
+  #addLayer(div, name) {
+    const pos = LAYERS_ORDER.get(name);
+    this.#layers[pos] = div;
+    for (let i = pos - 1; i >= 0; i--) {
+      const layer = this.#layers[i];
+      if (layer) {
+        layer.after(div);
+        return;
+      }
+    }
+    this.div.prepend(div);
+  }
+
   get renderingState() {
     return this.#renderingState;
   }
@@ -392,7 +415,7 @@ class PDFPageView {
       if (this.xfaLayer?.div) {
         // Pause translation when inserting the xfaLayer in the DOM.
         this.l10n.pause();
-        this.div.append(this.xfaLayer.div);
+        this.#addLayer(this.xfaLayer.div, "xfaLayer");
         this.l10n.resume();
       }
 
@@ -531,6 +554,10 @@ class PDFPageView {
           continue;
       }
       node.remove();
+      const layerIndex = this.#layers.indexOf(node);
+      if (layerIndex >= 0) {
+        this.#layers[layerIndex] = null;
+      }
     }
     div.removeAttribute("data-loaded");
 
@@ -877,7 +904,8 @@ class PDFPageView {
     // overflow will be hidden in Firefox.
     const canvasWrapper = document.createElement("div");
     canvasWrapper.classList.add("canvasWrapper");
-    div.append(canvasWrapper);
+    canvasWrapper.setAttribute("aria-hidden", true);
+    this.#addLayer(canvasWrapper, "canvasWrapper");
 
     if (
       !this.textLayer &&
@@ -891,13 +919,13 @@ class PDFPageView {
         accessibilityManager: this._accessibilityManager,
         enablePermissions:
           this.#textLayerMode === TextLayerMode.ENABLE_PERMISSIONS,
+        onAppend: textLayerDiv => {
+          // Pause translation when inserting the textLayer in the DOM.
+          this.l10n.pause();
+          this.#addLayer(textLayerDiv, "textLayer");
+          this.l10n.resume();
+        },
       });
-      this.textLayer.onAppend = textLayerDiv => {
-        // Pause translation when inserting the textLayer in the DOM.
-        this.l10n.pause();
-        this.div.append(textLayerDiv);
-        this.l10n.resume();
-      };
     }
 
     if (
@@ -915,7 +943,6 @@ class PDFPageView {
 
       this._annotationCanvasMap ||= new Map();
       this.annotationLayer = new AnnotationLayerBuilder({
-        pageDiv: div,
         pdfPage,
         annotationStorage,
         imageResourcesPath: this.imageResourcesPath,
@@ -927,6 +954,9 @@ class PDFPageView {
         fieldObjectsPromise,
         annotationCanvasMap: this._annotationCanvasMap,
         accessibilityManager: this._accessibilityManager,
+        onAppend: annotationLayerDiv => {
+          this.#addLayer(annotationLayerDiv, "annotationLayer");
+        },
       });
     }
 
@@ -1042,13 +1072,15 @@ class PDFPageView {
         if (!this.annotationEditorLayer) {
           this.annotationEditorLayer = new AnnotationEditorLayerBuilder({
             uiManager: annotationEditorUIManager,
-            pageDiv: div,
             pdfPage,
             l10n,
             accessibilityManager: this._accessibilityManager,
             annotationLayer: this.annotationLayer?.annotationLayer,
             textLayer: this.textLayer,
             drawLayer: this.drawLayer.getDrawLayer(),
+            onAppend: annotationEditorLayerDiv => {
+              this.#addLayer(annotationEditorLayerDiv, "annotationEditorLayer");
+            },
           });
         }
         this.#renderAnnotationEditorLayer();

web/pdf_viewer.css

@@ -75,7 +75,6 @@
   overflow: hidden;
   width: 100%;
   height: 100%;
-  z-index: 1;
 }
 
 .pdfViewer .page {

web/text_layer_builder.css

@@ -23,7 +23,6 @@
   text-size-adjust: none;
   forced-color-adjust: none;
   transform-origin: 0 0;
-  z-index: 2;
   caret-color: CanvasText;
 
   &.highlighting {

web/text_layer_builder.js

@@ -28,6 +28,7 @@ import { removeNullCharacters } from "./ui_utils.js";
  * @property {TextHighlighter} highlighter - Optional object that will handle
  *   highlighting text from the find controller.
  * @property {TextAccessibilityManager} [accessibilityManager]
+ * @property {function} [onAppend]
  */
 
 /**
@@ -38,6 +39,8 @@ import { removeNullCharacters } from "./ui_utils.js";
 class TextLayerBuilder {
   #enablePermissions = false;
 
+  #onAppend = null;
+
   #rotation = 0;
 
   #scale = 0;
@@ -48,6 +51,7 @@ class TextLayerBuilder {
     highlighter = null,
     accessibilityManager = null,
     enablePermissions = false,
+    onAppend = null,
   }) {
     this.textContentItemsStr = [];
     this.renderingDone = false;
@@ -57,14 +61,10 @@ class TextLayerBuilder {
     this.highlighter = highlighter;
     this.accessibilityManager = accessibilityManager;
     this.#enablePermissions = enablePermissions === true;
-
-    /**
-     * Callback used to attach the textLayer to the DOM.
-     * @type {function}
-     */
-    this.onAppend = null;
+    this.#onAppend = onAppend;
 
     this.div = document.createElement("div");
+    this.div.tabIndex = 0;
     this.div.className = "textLayer";
   }
 
@@ -132,7 +132,7 @@ class TextLayerBuilder {
     this.#rotation = rotation;
     // Ensure that the textLayer is appended to the DOM *before* handling
     // e.g. a pending search operation.
-    this.onAppend(this.div);
+    this.#onAppend?.(this.div);
     this.highlighter?.enable();
     this.accessibilityManager?.enable();
   }