frontend/src/assets/foliate/paginator.js (2)

1120-1126: ⚠️ Potential issue | 🟡 Minor

Guard unload path when open() hasn’t initialized sections yet.

At Line 1124, this.sections[this.#index]?.unload?.() throws if this.sections is undefined (e.g., element disconnected before open(book)).

✅ Minimal safe fix

-        this.sections[this.#index]?.unload?.()
+        this.sections?.[this.#index]?.unload?.()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/assets/foliate/paginator.js` around lines 1120 - 1126,
disconnectedCallback currently accesses this.sections[this.#index]?.unload?.()
which throws if this.sections is undefined (e.g., element disconnected before
open(book) ran); update disconnectedCallback (method name) to guard the path by
checking this.sections is truthy (and that this.#index is a valid index) before
calling unload on the chosen section, e.g., only call section.unload if
this.sections exists and the indexed section is defined; ensure you still
disconnect this.#observer, call this.#view?.disconnectedCallback(), set
this.#view = null, and remove the media query listener as before.

---

452-626: ⚠️ Potential issue | 🟠 Major

Prevent duplicate handlers on reconnect in connectedCallback().

connectedCallback() can run more than once per instance (Web Components lifecycle), and lines 567–574 add new bound listeners each time since .bind(this) creates a fresh function reference. disconnectedCallback() (lines 1120–1126) only removes the mediaQuery listener but leaves the host listeners intact. Reattaching the element adds duplicate touchstart, touchmove, and touchend handlers, causing navigation/selection to fire multiple times.

The same applies to the 'load' and 'relocate' listeners (lines 583–603) and the container scroll listeners (lines 567–572).

Fix by storing stable handler references as fields and guarding setup with an initialization flag:

♻️ Suggested fix

export class Paginator extends HTMLElement {
+    `#initialized` = false
+    `#onHostTouchStart` = e => this.#onTouchStart(e)
+    `#onHostTouchMove` = e => this.#onTouchMove(e)
+    `#onHostTouchEnd` = e => this.#onTouchEnd(e)
+    `#onMediaQueryChange` = () => {
+        if (!this.#view) return
+        this.#background.style.background = getBackground(this.#view.document)
+    }
+
     connectedCallback() {
+        if (this.#initialized) {
+            this.#observer.observe(this.#container)
+            this.#mediaQuery.addEventListener('change', this.#onMediaQueryChange)
+            return
+        }
+        this.#initialized = true
         // ... existing setup ...
-        this.addEventListener('touchstart', this.#onTouchStart.bind(this), opts)
-        this.addEventListener('touchmove', this.#onTouchMove.bind(this), opts)
-        this.addEventListener('touchend', this.#onTouchEnd.bind(this))
+        this.addEventListener('touchstart', this.#onHostTouchStart, opts)
+        this.addEventListener('touchmove', this.#onHostTouchMove, opts)
+        this.addEventListener('touchend', this.#onHostTouchEnd)
         // ... other listeners ...
-        this.#mediaQueryListener = () => {
-            if (!this.#view) return
-            this.#background.style.background = getBackground(this.#view.document)
-        }
-        this.#mediaQuery.addEventListener('change', this.#mediaQueryListener)
+        this.#mediaQuery.addEventListener('change', this.#onMediaQueryChange)
     }

     disconnectedCallback() {
         this.#observer.disconnect()
+        this.removeEventListener('touchstart', this.#onHostTouchStart)
+        this.removeEventListener('touchmove', this.#onHostTouchMove)
+        this.removeEventListener('touchend', this.#onHostTouchEnd)
+        this.removeEventListener('load', this.#onLoad)
+        this.removeEventListener('relocate', this.#onRelocate)
+        this.#container.removeEventListener('scroll', this.#onContainerScroll)
+        this.#container.removeEventListener('scroll', this.#onContainerScrollDebounced)
+        this.#mediaQuery.removeEventListener('change', this.#onMediaQueryChange)
         // ... existing cleanup ...
     }
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/assets/foliate/paginator.js` around lines 452 - 626,
connectedCallback currently binds and adds new anonymous listeners each time
(touchstart/touchmove/touchend, container scroll handlers, 'load' and 'relocate'
handlers and mediaQuery listener) which creates duplicates on reconnect; fix by
creating persistent bound handler fields (e.g. this._boundOnTouchStart =
this.#onTouchStart.bind(this), this._boundOnTouchMove, this._boundOnTouchEnd,
this._boundOnContainerScroll, this._boundOnContainerDebouncedScroll,
this._boundOnLoad, this._boundOnRelocate, this.#mediaQueryListener) and use
those fields when calling addEventListener in connectedCallback, add a guard
flag (e.g. this._initialized) to skip reattaching when already initialized, and
ensure disconnectedCallback removes the same stored handlers
(removeEventListener for touch, container scroll, load, relocate, and mediaQuery
change) and clears the flag so reattachment is safe.