[Editor] Add the possibility to change the thickness of a free highlight (bug 1876096)

l10n/en-US/viewer.ftl

@@ -349,6 +349,8 @@ pdfjs-editor-ink-opacity-input = Opacity
 pdfjs-editor-stamp-add-image-button =
     .title = Add image
 pdfjs-editor-stamp-add-image-button-label = Add image
+# This refers to the thickness of the line used for free highlighting (not bound to text)
+pdfjs-editor-free-highlight-thickness-input = Thickness
 
 pdfjs-free-text =
     .aria-label = Text Editor

src/display/draw_layer.js

@@ -155,6 +155,14 @@ class DrawLayer {
     path.setAttribute("d", line.toSVGPath());
   }
 
+  updateLine(id, line) {
+    const root = this.#mapping.get(id);
+    const defs = root.firstChild;
+    const path = defs.firstChild;
+    this.updateBox(id, line.box);
+    path.setAttribute("d", line.toSVGPath());
+  }
+
   removeFreeHighlight(id) {
     this.remove(id);
     this.#toUpdate.delete(id);

src/display/editor/annotation_editor_layer.js

@@ -359,7 +359,11 @@ class AnnotationEditorLayer {
         // Do nothing on right click.
         return;
       }
-      HighlightEditor.startHighlighting(this, event);
+      HighlightEditor.startHighlighting(
+        this,
+        this.#uiManager.direction === "ltr",
+        event
+      );
       event.preventDefault();
     }
   }

src/display/editor/editor.js

@@ -472,7 +472,7 @@ class AnnotationEditor {
     // the position: it'll be done when the user will release the mouse button.
 
     let { x, y } = this;
-    const [bx, by] = this.#getBaseTranslation();
+    const [bx, by] = this.getBaseTranslation();
     x += bx;
     y += by;
 
@@ -481,7 +481,14 @@ class AnnotationEditor {
     this.div.scrollIntoView({ block: "nearest" });
   }
 
-  #getBaseTranslation() {
+  /**
+   * Get the translation to take into account the editor border.
+   * The CSS engine positions the element by taking the border into account so
+   * we must apply the opposite translation to have the editor in the right
+   * position.
+   * @returns {Array<number>}
+   */
+  getBaseTranslation() {
     const [parentWidth, parentHeight] = this.parentDimensions;
     const { _borderLineWidth } = AnnotationEditor;
     const x = _borderLineWidth / parentWidth;
@@ -532,7 +539,7 @@ class AnnotationEditor {
     this.x = x /= pageWidth;
     this.y = y /= pageHeight;
 
-    const [bx, by] = this.#getBaseTranslation();
+    const [bx, by] = this.getBaseTranslation();
     x += bx;
     y += by;
 

src/display/editor/highlight.js

@@ -50,11 +50,13 @@ class HighlightEditor extends AnnotationEditor {
 
   #outlineId = null;
 
+  #thickness;
+
   static _defaultColor = null;
 
   static _defaultOpacity = 1;
 
-  static _defaultThickness = 10;
+  static _defaultThickness = 12;
 
   static _l10nPromise;
 
@@ -71,6 +73,7 @@ class HighlightEditor extends AnnotationEditor {
   constructor(params) {
     super({ ...params, name: "highlightEditor" });
     this.color = params.color || HighlightEditor._defaultColor;
+    this.#thickness = params.thickness || HighlightEditor._defaultThickness;
     this.#opacity = params.opacity || HighlightEditor._defaultOpacity;
     this.#boxes = params.boxes || null;
     this._isDraggable = false;
@@ -112,17 +115,31 @@ class HighlightEditor extends AnnotationEditor {
     ];
   }
 
-  #createFreeOutlines({ highlight, highlightId, clipPathId }) {
+  #createFreeOutlines({ highlightOutlines, highlightId, clipPathId }) {
-    this.#highlightOutlines = highlight.getOutlines(
+    this.#highlightOutlines = highlightOutlines;
-      this._uiManager.direction === "ltr"
+    const extraThickness = 1.5;
+    this.#focusOutlines = highlightOutlines.getNewOutline(
+      /* Slightly bigger than the highlight in order to have a little
+         space between the highlight and the outline. */
+      this.#thickness / 2 + extraThickness,
+      /* innerMargin = */ 0.0025
     );
-    this.#id = highlightId;
-    this.#clipPathId = clipPathId;
-    const { x, y, width, height, lastPoint } = this.#highlightOutlines.box;
 
-    // We need to redraw the highlight because we change the coordinates to be
+    if (highlightId >= 0) {
-    // in the box coordinate system.
+      this.#id = highlightId;
-    this.parent.drawLayer.finalizeLine(this.#id, this.#highlightOutlines);
+      this.#clipPathId = clipPathId;
+      // We need to redraw the highlight because we change the coordinates to be
+      // in the box coordinate system.
+      this.parent.drawLayer.finalizeLine(highlightId, highlightOutlines);
+      this.#outlineId = this.parent.drawLayer.highlightOutline(
+        this.#focusOutlines
+      );
+    } else if (this.parent) {
+      this.parent.drawLayer.updateLine(this.#id, highlightOutlines);
+      this.parent.drawLayer.updateLine(this.#outlineId, this.#focusOutlines);
+    }
+    const { x, y, width, height, lastPoint } = highlightOutlines.box;
+    this.#lastPoint = lastPoint;
     switch (this.rotation) {
       case 0:
         this.x = x;
@@ -153,30 +170,24 @@ class HighlightEditor extends AnnotationEditor {
         break;
       }
     }
-
-    const innerMargin = 1.5;
-    this.#focusOutlines = highlight.getFocusOutline(
-      /* Slightly bigger than the highlight in order to have a little
-         space between the highlight and the outline. */
-      HighlightEditor._defaultThickness + innerMargin
-    );
-    this.#outlineId = this.parent.drawLayer.highlightOutline(
-      this.#focusOutlines
-    );
-    this.#lastPoint = lastPoint;
   }
 
+  /** @inheritdoc */
   static initialize(l10n, uiManager) {
     AnnotationEditor.initialize(l10n, uiManager);
     HighlightEditor._defaultColor ||=
       uiManager.highlightColors?.values().next().value || "#fff066";
   }
 
+  /** @inheritdoc */
   static updateDefaultParams(type, value) {
     switch (type) {
       case AnnotationEditorParamsType.HIGHLIGHT_DEFAULT_COLOR:
         HighlightEditor._defaultColor = value;
         break;
+      case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS:
+        HighlightEditor._defaultThickness = value;
+        break;
     }
   }
 
@@ -194,6 +205,9 @@ class HighlightEditor extends AnnotationEditor {
       case AnnotationEditorParamsType.HIGHLIGHT_COLOR:
         this.#updateColor(value);
         break;
+      case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS:
+        this.#updateThickness(value);
+        break;
     }
   }
 
@@ -203,6 +217,10 @@ class HighlightEditor extends AnnotationEditor {
         AnnotationEditorParamsType.HIGHLIGHT_DEFAULT_COLOR,
         HighlightEditor._defaultColor,
       ],
+      [
+        AnnotationEditorParamsType.HIGHLIGHT_THICKNESS,
+        HighlightEditor._defaultThickness,
+      ],
     ];
   }
 
@@ -213,6 +231,10 @@ class HighlightEditor extends AnnotationEditor {
         AnnotationEditorParamsType.HIGHLIGHT_COLOR,
         this.color || HighlightEditor._defaultColor,
       ],
+      [
+        AnnotationEditorParamsType.HIGHLIGHT_THICKNESS,
+        this.#thickness || HighlightEditor._defaultThickness,
+      ],
     ];
   }
 
@@ -238,6 +260,27 @@ class HighlightEditor extends AnnotationEditor {
     });
   }
 
+  /**
+   * Update the thickness and make this action undoable.
+   * @param {number} thickness
+   */
+  #updateThickness(thickness) {
+    const savedThickness = this.#thickness;
+    const setThickness = th => {
+      this.#thickness = th;
+      this.#changeThickness(th);
+    };
+    this.addCommands({
+      cmd: setThickness.bind(this, thickness),
+      undo: setThickness.bind(this, savedThickness),
+      post: this._uiManager.updateUI.bind(this._uiManager, this),
+      mustExec: true,
+      type: AnnotationEditorParamsType.INK_THICKNESS,
+      overwriteIfSameType: true,
+      keepUndo: true,
+    });
+  }
+
   /** @inheritdoc */
   async addEditToolbar() {
     const toolbar = await super.addEditToolbar();
@@ -268,6 +311,13 @@ class HighlightEditor extends AnnotationEditor {
     return super.fixAndSetPosition(this.#getRotation());
   }
 
+  /** @inheritdoc */
+  getBaseTranslation() {
+    // The editor itself doesn't have any CSS border (we're drawing one
+    // ourselves in using SVG).
+    return [0, 0];
+  }
+
   /** @inheritdoc */
   getRect(tx, ty) {
     return super.getRect(tx, ty, this.#getRotation());
@@ -322,6 +372,18 @@ class HighlightEditor extends AnnotationEditor {
     }
   }
 
+  #changeThickness(thickness) {
+    if (!this.#isFreeHighlight) {
+      return;
+    }
+    this.#createFreeOutlines({
+      highlightOutlines: this.#highlightOutlines.getNewOutline(thickness / 2),
+    });
+    this.fixAndSetPosition();
+    const [parentWidth, parentHeight] = this.parentDimensions;
+    this.setDims(this.width * parentWidth, this.height * parentHeight);
+  }
+
   #cleanDrawLayer() {
     if (this.#id === null || !this.parent) {
       return;
@@ -480,7 +542,7 @@ class HighlightEditor extends AnnotationEditor {
     return this.#highlightOutlines.serialize(rect, this.#getRotation());
   }
 
-  static startHighlighting(parent, { target: textLayer, x, y }) {
+  static startHighlighting(parent, isLTR, { target: textLayer, x, y }) {
     const {
       x: layerX,
       y: layerY,
@@ -518,7 +580,8 @@ class HighlightEditor extends AnnotationEditor {
       { x, y },
       [layerX, layerY, parentWidth, parentHeight],
       parent.scale,
-      this._defaultThickness,
+      this._defaultThickness / 2,
+      isLTR,
       /* innerMargin = */ 0.001
     );
     ({ id: this._freeHighlightId, clipPathId: this._freeHighlightClipId } =
@@ -541,7 +604,7 @@ class HighlightEditor extends AnnotationEditor {
     if (!this._freeHighlight.isEmpty()) {
       parent.createAndAddNewEditor(event, false, {
         highlightId: this._freeHighlightId,
-        highlight: this._freeHighlight,
+        highlightOutlines: this._freeHighlight.getOutlines(),
         clipPathId: this._freeHighlightClipId,
       });
     } else {
@@ -595,7 +658,7 @@ class HighlightEditor extends AnnotationEditor {
       annotationType: AnnotationEditorType.HIGHLIGHT,
       color,
       opacity: this.#opacity,
-      thickness: 2 * HighlightEditor._defaultThickness,
+      thickness: this.#thickness,
       quadPoints: this.#serializeBoxes(rect),
       outlines: this.#serializeOutlines(rect),
       pageIndex: this.pageIndex,

src/display/editor/outliner.js

@@ -350,6 +350,8 @@ class FreeOutliner {
 
   #innerMargin;
 
+  #isLTR;
+
   #top = [];
 
   // The first 6 elements are the last 3 points of the top part of the outline.
@@ -377,9 +379,10 @@ class FreeOutliner {
 
   static #MIN = FreeOutliner.#MIN_DIST + FreeOutliner.#MIN_DIFF;
 
-  constructor({ x, y }, box, scaleFactor, thickness, innerMargin = 0) {
+  constructor({ x, y }, box, scaleFactor, thickness, isLTR, innerMargin = 0) {
     this.#box = box;
     this.#thickness = thickness * scaleFactor;
+    this.#isLTR = isLTR;
     this.#last.set([NaN, NaN, NaN, NaN, x, y], 6);
     this.#innerMargin = innerMargin;
     this.#min_dist = FreeOutliner.#MIN_DIST * scaleFactor;
@@ -571,24 +574,7 @@ class FreeOutliner {
     return buffer.join(" ");
   }
 
-  getFocusOutline(thickness) {
+  getOutlines() {
-    // Build the outline of the highlight to use as the focus outline.
-    const [x, y] = this.#points;
-    const outliner = new FreeOutliner(
-      { x, y },
-      this.#box,
-      this.#scaleFactor,
-      thickness,
-      /* innerMargin = */ 0.0025
-    );
-    outliner.#points = null;
-    for (let i = 2; i < this.#points.length; i += 2) {
-      outliner.add({ x: this.#points[i], y: this.#points[i + 1] });
-    }
-    return outliner.getOutlines();
-  }
-
-  getOutlines(isLTR) {
     const top = this.#top;
     const bottom = this.#bottom;
     const last = this.#last;
@@ -637,8 +623,10 @@ class FreeOutliner {
       return new FreeHighlightOutline(
         outline,
         points,
+        this.#box,
+        this.#scaleFactor,
         this.#innerMargin,
-        isLTR
+        this.#isLTR
       );
     }
 
@@ -686,24 +674,40 @@ class FreeOutliner {
       }
     }
     outline.set([NaN, NaN, NaN, NaN, bottom[4], bottom[5]], N);
-    return new FreeHighlightOutline(outline, points, this.#innerMargin, isLTR);
+    return new FreeHighlightOutline(
+      outline,
+      points,
+      this.#box,
+      this.#scaleFactor,
+      this.#innerMargin,
+      this.#isLTR
+    );
   }
 }
 
 class FreeHighlightOutline extends Outline {
+  #box;
+
   #bbox = null;
 
   #innerMargin;
 
+  #isLTR;
+
   #points;
 
+  #scaleFactor;
+
   #outline;
 
-  constructor(outline, points, innerMargin, isLTR) {
+  constructor(outline, points, box, scaleFactor, innerMargin, isLTR) {
     super();
     this.#outline = outline;
     this.#points = points;
+    this.#box = box;
+    this.#scaleFactor = scaleFactor;
     this.#innerMargin = innerMargin;
+    this.#isLTR = isLTR;
     this.#computeMinMax(isLTR);
 
     const { x, y, width, height } = this.#bbox;
@@ -841,6 +845,34 @@ class FreeHighlightOutline extends Outline {
   get box() {
     return this.#bbox;
   }
+
+  getNewOutline(thickness, innerMargin) {
+    // Build the outline of the highlight to use as the focus outline.
+    const { x, y, width, height } = this.#bbox;
+    const [layerX, layerY, layerWidth, layerHeight] = this.#box;
+    const sx = width * layerWidth;
+    const sy = height * layerHeight;
+    const tx = x * layerWidth + layerX;
+    const ty = y * layerHeight + layerY;
+    const outliner = new FreeOutliner(
+      {
+        x: this.#points[0] * sx + tx,
+        y: this.#points[1] * sy + ty,
+      },
+      this.#box,
+      this.#scaleFactor,
+      thickness,
+      this.#isLTR,
+      innerMargin ?? this.#innerMargin
+    );
+    for (let i = 2; i < this.#points.length; i += 2) {
+      outliner.add({
+        x: this.#points[i] * sx + tx,
+        y: this.#points[i + 1] * sy + ty,
+      });
+    }
+    return outliner.getOutlines();
+  }
 }
 
 export { FreeOutliner, Outliner };

src/shared/util.js

@@ -88,6 +88,7 @@ const AnnotationEditorParamsType = {
   INK_OPACITY: 23,
   HIGHLIGHT_COLOR: 31,
   HIGHLIGHT_DEFAULT_COLOR: 32,
+  HIGHLIGHT_THICKNESS: 33,
 };
 
 // Permission flags from Table 22, Section 7.6.3.2 of the PDF specification.

test/integration/highlight_editor_spec.mjs

@@ -14,7 +14,9 @@
  */
 
 import {
+  awaitPromise,
   closePages,
+  createPromise,
   getEditorSelector,
   getSerialized,
   kbBigMoveLeft,
@@ -32,6 +34,11 @@ const selectAll = async page => {
   );
 };
 
+const waitForPointerUp = page =>
+  createPromise(page, resolve => {
+    window.addEventListener("pointerup", resolve, { once: true });
+  });
+
 const getXY = (page, selector) =>
   page.evaluate(sel => {
     const bbox = document.querySelector(sel).getBoundingClientRect();
@@ -586,4 +593,92 @@ describe("Highlight Editor", () => {
       );
     });
   });
+
+  describe("Free highlight thickness can be changed", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait(
+        "empty.pdf",
+        ".annotationEditorLayer",
+        null,
+        null,
+        { highlightEditorColors: "yellow=#000000" }
+      );
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that the thickness is correctly updated", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#editorHighlight");
+          await page.waitForSelector(".annotationEditorLayer.highlightEditing");
+
+          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 + 120 + i * 120;
+            const y = rect.y + 120 + i * 120;
+            const clickHandle = await waitForPointerUp(page);
+            await page.mouse.move(x, y);
+            await page.mouse.down();
+            await page.mouse.move(x + 100, y + 100);
+            await page.mouse.up();
+            await awaitPromise(clickHandle);
+
+            await page.waitForSelector(getEditorSelector(i));
+          }
+
+          let value = 12;
+          await waitForSerialized(page, 3);
+          let serialized = await getSerialized(page);
+          expect(serialized.map(x => x.thickness))
+            .withContext(`In ${browserName}`)
+            .toEqual([value, value, value]);
+
+          await selectAll(page);
+
+          const prevWidth = await page.evaluate(
+            sel => document.querySelector(sel).getBoundingClientRect().width,
+            getEditorSelector(0)
+          );
+
+          value = 24;
+          page.evaluate(val => {
+            window.PDFViewerApplication.eventBus.dispatch(
+              "switchannotationeditorparams",
+              {
+                source: null,
+                type: window.pdfjsLib.AnnotationEditorParamsType
+                  .HIGHLIGHT_THICKNESS,
+                value: val,
+              }
+            );
+          }, value);
+
+          await page.waitForFunction(
+            (w, sel) =>
+              document.querySelector(sel).getBoundingClientRect().width !== w,
+            {},
+            prevWidth,
+            getEditorSelector(0)
+          );
+
+          await waitForSerialized(page, 3);
+          serialized = await getSerialized(page);
+          expect(serialized.map(x => x.thickness))
+            .withContext(`In ${browserName}`)
+            .toEqual([value, value, value]);
+        })
+      );
+    });
+  });
 });

web/annotation_editor_layer_builder.css

@@ -1094,20 +1094,21 @@
   height: auto;
   padding-inline: 10px;
   padding-block: 10px 16px;
+  gap: 16px;
   display: flex;
   flex-direction: column;
   box-sizing: border-box;
 
+  .editorParamsLabel {
+    width: fit-content;
+    inset-inline-start: 0;
+  }
+
   .colorPicker {
     display: flex;
     flex-direction: column;
     gap: 8px;
 
-    #highlightColorPickerLabel {
-      width: fit-content;
-      inset-inline-start: 0;
-    }
-
     .dropdown {
       display: flex;
       justify-content: space-between;
@@ -1145,4 +1146,58 @@
       }
     }
   }
+
+  #editorHighlightThickness {
+    display: flex;
+    flex-direction: column;
+    align-items: center;
+    gap: 4px;
+    align-self: stretch;
+
+    .editorParamsLabel {
+      width: 100%;
+      height: auto;
+      align-self: stretch;
+    }
+
+    .thicknessPicker {
+      display: flex;
+      justify-content: space-between;
+      align-items: center;
+      align-self: stretch;
+
+      --example-color: #bfbfc9;
+
+      @media (prefers-color-scheme: dark) {
+        --example-color: #80808e;
+      }
+
+      @media screen and (forced-colors: active) {
+        --example-color: HighlightText;
+      }
+
+      &::before {
+        content: "";
+        width: 8px;
+        aspect-ratio: 1;
+        display: block;
+        border-radius: 100%;
+        background-color: var(--example-color);
+      }
+
+      .editorParamsSlider {
+        width: unset;
+        height: 14px;
+      }
+
+      &::after {
+        content: "";
+        width: 24px;
+        aspect-ratio: 1;
+        display: block;
+        border-radius: 100%;
+        background-color: var(--example-color);
+      }
+    }
+  }
 }

web/annotation_editor_params.js

@@ -32,6 +32,7 @@ class AnnotationEditorParams {
     editorInkThickness,
     editorInkOpacity,
     editorStampAddImage,
+    editorFreeHighlightThickness,
   }) {
     const dispatchEvent = (typeStr, value) => {
       this.eventBus.dispatch("switchannotationeditorparams", {
@@ -58,6 +59,9 @@ class AnnotationEditorParams {
     editorStampAddImage.addEventListener("click", () => {
       dispatchEvent("CREATE");
     });
+    editorFreeHighlightThickness.addEventListener("input", function () {
+      dispatchEvent("HIGHLIGHT_THICKNESS", this.valueAsNumber);
+    });
 
     this.eventBus._on("annotationeditorparamschanged", evt => {
       for (const [type, value] of evt.details) {
@@ -77,6 +81,9 @@ class AnnotationEditorParams {
           case AnnotationEditorParamsType.INK_OPACITY:
             editorInkOpacity.value = value;
             break;
+          case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS:
+            editorFreeHighlightThickness.value = value;
+            break;
         }
       }
     });

web/viewer.html

@@ -176,6 +176,12 @@ See https://github.com/adobe-type-tools/cmap-resources
             <div id="editorHighlightColorPicker" class="colorPicker">
               <span id="highlightColorPickerLabel" class="editorParamsLabel" data-l10n-id="pdfjs-editor-highlight-colorpicker-label">Highlight color</span>
             </div>
+            <div id="editorHighlightThickness">
+              <label for="editorFreeHighlightThickness" class="editorParamsLabel" data-l10n-id="pdfjs-editor-free-highlight-thickness-input">Thickness</label>
+              <div class="thicknessPicker">
+                <input type="range" id="editorFreeHighlightThickness" class="editorParamsSlider" value="12" min="8" max="24" step="1" tabindex="101">
+              </div>
+            </div>
           </div>
         </div>
 

web/viewer.js

@@ -172,6 +172,9 @@ function getViewerConfiguration() {
       editorInkThickness: document.getElementById("editorInkThickness"),
       editorInkOpacity: document.getElementById("editorInkOpacity"),
       editorStampAddImage: document.getElementById("editorStampAddImage"),
+      editorFreeHighlightThickness: document.getElementById(
+        "editorFreeHighlightThickness"
+      ),
     },
     printContainer: document.getElementById("printContainer"),
     openFileInput: