fix(pdf-server): import highlight/underline/strike from existing PDFs (#592)

* fix(pdf-server): import highlight/underline/strike from existing PDFs

importPdfjsAnnotation expected nested quadPoints arrays, but pdf.js
emits a flat Float32Array (8 numbers per quad). Iterating it yielded
numbers, qp.length was undefined, rects stayed empty, and the function
returned null - so every quad-based annotation in a loaded PDF was
dropped from annotationMap and rendered only as unselectable canvas
pixels. Shipped this way in #506; not a regression.

Parse the flat array in 8-stride chunks, fall through to ann.rect if
that yields nothing. Existing tests used the (wrong) nested fixture
shape; switched them to Float32Array and added a 2-quad case + a
rect-fallback case.

* fix(pdf-server): own markup annotations in our layer, not the canvas

page.render() defaulted to AnnotationMode.ENABLE, painting every
annotation's appearance stream onto the canvas. We ALSO import them into
annotationMap and render DOM versions in #annotation-layer, so the user
saw two representations: an unclickable canvas pixel that looked right,
and our clickable DOM element that looked like our styling. Clicking the
'real' one did nothing.

Set annotationMode: DISABLE so the canvas is page-content-only and our
layer is the single source of truth for markup. Imported annotations now
behave identically to user-created ones (select/drag/delete). Form
widgets are unaffected (#form-layer via AnnotationLayer.render).
get_screenshot keeps ENABLE_STORAGE so screenshots still show
annotations.

Known trade-offs:
- Stamps with image appearance streams degrade to our text-label render
  (no rasterization yet).
- Annotation types we don't import (Ink, Polygon, Caret, ...) no longer
  ghost on the canvas - they're invisible. They were never editable
  anyway; loadBaselineAnnotations already logs them.

* Revert "fix(pdf-server): own markup annotations in our layer, not the canvas"

This reverts commit 2f989a757d3aa517fa427c7fd59b462326599bfc.
This commit is contained in:
Olivier Chafik 2026-04-02 10:00:53 +01:00 • committed by GitHub
commit f3bf1065db
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 71 additions and 29 deletions

View file

@ -460,7 +460,8 @@ describe("importPdfjsAnnotation", () => {
annotationType: 9,
ref: { num: 5, gen: 0 },
rect: [72, 700, 272, 712],
quadPoints: [[72, 712, 272, 712, 72, 700, 272, 700]],
// pdf.js emits a FLAT Float32Array, not nested arrays.
quadPoints: new Float32Array([72, 712, 272, 712, 72, 700, 272, 700]),
color: new Uint8ClampedArray([255, 255, 0]),
contentsObj: { str: "Important" },
};
@ -474,12 +475,57 @@ describe("importPdfjsAnnotation", () => {
expect((result as any).color).toBe("#ffff00");
});
it("imports a multi-line highlight (multiple quads → multiple rects)", () => {
// Regression: the parser iterated quadPoints as if nested; pdf.js's
// flat array yielded numbers, so rects stayed empty and import bailed.
const ann = {
annotationType: 9,
ref: { num: 8, gen: 0 },
quadPoints: new Float32Array([
// line 1
72, 712, 272, 712, 72, 700, 272, 700,
// line 2
72, 698, 200, 698, 72, 686, 200, 686,
]),
color: new Uint8ClampedArray([255, 255, 0]),
};
const result = importPdfjsAnnotation(ann, 1, 0);
expect(result).not.toBeNull();
expect(result!.type).toBe("highlight");
expect((result as any).rects).toHaveLength(2);
expect((result as any).rects[0]).toEqual({
x: 72,
y: 700,
width: 200,
height: 12,
});
expect((result as any).rects[1]).toEqual({
x: 72,
y: 686,
width: 128,
height: 12,
});
});
it("falls back to ann.rect when quadPoints is absent", () => {
const ann = {
annotationType: 9,
ref: { num: 9, gen: 0 },
rect: [72, 700, 272, 712],
color: new Uint8ClampedArray([255, 255, 0]),
};
const result = importPdfjsAnnotation(ann, 1, 0);
expect(result).not.toBeNull();
expect((result as any).rects).toHaveLength(1);
});
it("imports an underline annotation", () => {
const ann = {
annotationType: 10,
ref: { num: 6, gen: 0 },
rect: [72, 700, 272, 712],
quadPoints: [[72, 712, 272, 712, 72, 700, 272, 700]],
// pdf.js emits a FLAT Float32Array, not nested arrays.
quadPoints: new Float32Array([72, 712, 272, 712, 72, 700, 272, 700]),
color: new Uint8ClampedArray([255, 0, 0]),
};
const result = importPdfjsAnnotation(ann, 1, 0);
@ -493,7 +539,8 @@ describe("importPdfjsAnnotation", () => {
annotationType: 12,
ref: { num: 7, gen: 0 },
rect: [72, 700, 272, 712],
quadPoints: [[72, 712, 272, 712, 72, 700, 272, 700]],
// pdf.js emits a FLAT Float32Array, not nested arrays.
quadPoints: new Float32Array([72, 712, 272, 712, 72, 700, 272, 700]),
color: new Uint8ClampedArray([255, 0, 0]),
};
const result = importPdfjsAnnotation(ann, 2, 0);

View file

@ -948,33 +948,28 @@ export function importPdfjsAnnotation(
case "highlight":
case "underline":
case "strikethrough": {
// PDF.js provides quadPoints as array of arrays [[x1,y1,x2,y2,...], ...]
// or rect as [x1,y1,x2,y2]
let rects: Rect[];
if (ann.quadPoints && ann.quadPoints.length > 0) {
rects = [];
for (const qp of ann.quadPoints) {
// Each quadPoint is [x1,y1,x2,y2,x3,y3,x4,y4]
// We need the bounding box
if (qp.length >= 8) {
const xs = [qp[0], qp[2], qp[4], qp[6]];
const ys = [qp[1], qp[3], qp[5], qp[7]];
const minX = Math.min(...xs);
const minY = Math.min(...ys);
const maxX = Math.max(...xs);
const maxY = Math.max(...ys);
rects.push({
x: minX,
y: minY,
width: maxX - minX,
height: maxY - minY,
});
}
// pdf.js emits quadPoints as a FLAT Float32Array [x1,y1,...,x4,y4, …]
// (8 numbers per quad), NOT as nested arrays. Iterating it yields
// numbers, so the old `for (const qp of …) if (qp.length>=8)` never
// matched and every quad-based annotation was dropped (#506).
const rects: Rect[] = [];
const qp = ann.quadPoints as ArrayLike<number> | undefined;
if (qp && qp.length >= 8) {
for (let i = 0; i + 8 <= qp.length; i += 8) {
const xs = [qp[i], qp[i + 2], qp[i + 4], qp[i + 6]];
const ys = [qp[i + 1], qp[i + 3], qp[i + 5], qp[i + 7]];
const minX = Math.min(...xs);
const minY = Math.min(...ys);
rects.push({
x: minX,
y: minY,
width: Math.max(...xs) - minX,
height: Math.max(...ys) - minY,
});
}
} else if (ann.rect) {
rects = [pdfjsRectToRect(ann.rect)];
} else {
return null;
}
if (rects.length === 0 && ann.rect) {
rects.push(pdfjsRectToRect(ann.rect));
}
if (rects.length === 0) return null;