From 34a27a5951470af696d1ad53e98a496d4c480cd5 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Mon, 17 Aug 2026 08:33:11 -0400 Subject: [PATCH] =?UTF-8?q?fix(reader):=20PDF=20highlights=20offset=20from?= =?UTF-8?q?=20the=20words=20=E2=80=94=20wrong=20fraction=20denominator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Highlights landed on the right line but shifted right and oversized on any display with devicePixelRatio != 1. Cause: selection fractions divided the textLayer span rects by documentElement's screen rect, but pdf.js scales the iframe's by 1/dpr — that rect is dpr× smaller than the visible page, inflating every x/w fraction by dpr (on a 2× display a highlight started twice as far right and was twice as wide). dpr=1 displays were coincidentally correct, which is why the geometry looked sound when written. The denominator is now the rendered canvas (#canvas canvas), whose post-transform rect IS the visible page and shares the textLayer's transform space — the dpr scaling cancels exactly. Comics keep the img denominator; a viewport fallback covers any page without either. The popover-placement scale factors (frame/denominator) become 1 for PDFs as a side effect, fixing popover drift too. The fork's click hit-test (86e234d) gets the same canvas-aware denominator so clicking highlights opens the editor at the right spot. Highlights saved before this fix stored dpr-inflated fractions and will still render misplaced — delete and re-create them. --- package.json | 2 +- web/src/reader/reader.ts | 32 +++++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 42d8583..d1354c2 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "dev": "npm run build:ts:dev && npm run build:css" }, "dependencies": { - "@bookhoard/foliate-js": "git+https://github.com/john-okeefe/foliate-js.git#d065495", + "@bookhoard/foliate-js": "git+https://github.com/john-okeefe/foliate-js.git#86e234d", "alpinejs": "^3.15.8", "chart.js": "^4.5.1", "highlight.js": "^11.11.1", diff --git a/web/src/reader/reader.ts b/web/src/reader/reader.ts index 8e97401..ad6fde8 100644 --- a/web/src/reader/reader.ts +++ b/web/src/reader/reader.ts @@ -702,14 +702,26 @@ document.addEventListener("alpine:init", () => { const range = sel.getRangeAt(0); const text = sel.toString().replace(/\s+/g, " ").trim(); if (!text) return; - // Denominator in the same (transform-inclusive) coordinate - // space as getClientRects so the devicePixelRatio transform - // pdf.js applies to cancels in the fraction. + // Denominator: the element whose post-transform screen rect IS + // the visible page. For PDFs that's the rendered canvas — pdf.js + // scales by 1/devicePixelRatio, so documentElement's rect + // is dpr× too small and would inflate every fraction (highlight + // shifted right/oversized on any dpr != 1 display). The canvas's + // rect is in the same transform-inclusive space as the textLayer + // span rects, so the dpr scaling cancels exactly. const denom = - (doc.querySelector("img") as HTMLElement) || - doc.documentElement; - const dr = denom.getBoundingClientRect(); - if (!dr.width || !dr.height) return; + (doc.querySelector("#canvas canvas") as HTMLElement) || + (doc.querySelector("img") as HTMLElement); + let dr = denom?.getBoundingClientRect(); + if (!dr || !dr.width || !dr.height) { + const vw = doc.defaultView; + dr = { + left: 0, + top: 0, + width: vw?.innerWidth || 1, + height: vw?.innerHeight || 1, + } as DOMRect; + } const rects: number[][] = []; for (const r of range.getClientRects()) { const x = (r.left - dr.left) / dr.width; @@ -719,8 +731,10 @@ document.addEventListener("alpine:init", () => { if (w > 0 && h > 0) rects.push([x, y, w, h]); } if (!rects.length) return; - // Map the first rect to host-space for popover placement, - // accounting for the iframe's own scale factor. + // Map the first rect to host-space for popover placement. + // The canvas's screen rect maps 1:1 onto the host iframe box + // (the visible page fills the iframe), so sx/sy are 1 for PDFs; + // kept general for the comic img fallback. const frame = doc.defaultView?.frameElement as HTMLElement | null; if (!frame) return; const fr = frame.getBoundingClientRect();