From 2366faccce3a4ca080ed38b8ff1f76730cf65b7a Mon Sep 17 00:00:00 2001
From: John O'Keefe <jrokeefe@protonmail.com>
Date: Wed, 9 Sep 2026 12:38:24 -0400
Subject: [PATCH] fix(reader): make device-synced highlights editable on the
 web
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Device-synced highlights paint with a synthesized range CFI (renderCfi,
built by toRangeCfi from the stored point CFIs and selection text) while
the stored locator stays a point CFI. Three follow-ons from that split:

- show-annotation (click-to-edit) matched the clicked value against the
  stored point cfi only, so clicking a device-created highlight never
  opened the edit popover — it listed in the drawer but was uneditable.
  Match either the stored cfi or the renderCfi the overlay was added by.

- deleteHighlightById removed the overlay with the stored point cfi,
  which never matched the painted value; the highlight box lingered
  until reload. Delete with the value it was added by.

- saveHighlightChanges re-added the overlay without removing the old
  value; an edit that changes the synthesized range (note/text edits
  change the UTF-16 length it derives from) would ghost the old paint
  beside the new one. Remove the previous overlay value first when the
  edit changed it.

Web-created highlights are unaffected: their stored CFI is already a
native range, so renderCfi === cfi for them.
---
 web/src/reader/reader.ts | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/web/src/reader/reader.ts b/web/src/reader/reader.ts
index 71598df..805e99b 100644
--- a/web/src/reader/reader.ts
+++ b/web/src/reader/reader.ts
@@ -826,7 +826,13 @@ document.addEventListener("alpine:init", () => {
       });
       this.view.addEventListener("show-annotation", (e: any) => {
         const { value, index, range } = e.detail;
-        const h = this.highlightItems.find((x) => x.cfi === value);
+        // Device-synced highlights are painted with a synthesized range
+        // CFI (renderCfi) while the stored locator stays a point CFI, so a
+        // click reports the render value — match either or editing
+        // device highlights is impossible.
+        const h = this.highlightItems.find(
+          (x) => x.cfi === value || x.renderCfi === value,
+        );
         if (!h) return;
         const doc = this.renderer
           ?.getContents?.()
@@ -1343,7 +1349,23 @@ document.addEventListener("alpine:init", () => {
         if (!resp.ok) return;
         const row = await resp.json();
         const idx = this.highlightItems.findIndex((h) => h.id === p.id);
+        // The overlay is keyed by the value it was added with; an edit can
+        // change it (note/text edits change the synthesized range), so
+        // remove the old paint before re-adding or it ghosts.
+        const oldValue =
+          idx !== -1
+            ? this.highlightItems[idx].renderCfi ||
+              this.highlightItems[idx].cfi
+            : "";
         if (idx !== -1) this.highlightItems[idx] = this.mapHighlightRow(row);
+        const newValue =
+          idx !== -1
+            ? this.highlightItems[idx].renderCfi ||
+              this.highlightItems[idx].cfi
+            : "";
+        if (p.pdfPage < 0 && oldValue && oldValue !== newValue) {
+          this.view?.deleteAnnotation({ value: oldValue });
+        }
         // Re-add so the overlay redraws with the new color.
         if (p.pdfPage >= 0) {
           this.renderer?.addRectAnnotation?.({
@@ -1379,7 +1401,11 @@ document.addEventListener("alpine:init", () => {
         if (hl?.pdfPage >= 0) {
           this.renderer?.removeRectAnnotation?.(id);
         } else if (hl?.cfi) {
-          this.view?.deleteAnnotation({ value: hl.cfi });
+          // Delete with the value the overlay was added by: device-synced
+          // highlights paint a synthesized range, not the stored point CFI.
+          this.view?.deleteAnnotation({
+            value: hl.renderCfi || hl.cfi,
+          });
         }
         this.hideSelectionPopover();
       } catch (_e) {