From 5e06e77d7fa47876122d32336bf168ef680c129f Mon Sep 17 00:00:00 2001
From: librelad <librelad@digitalangels.vip>
Date: Wed, 12 Aug 2026 22:40:26 +0100
Subject: [PATCH] fix(forms): one bad select no longer kills every dropdown
 after it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reported: dropdowns dead on the app config page only, in a private window
(so not cache), with the served frontend confirmed identical to source.

The enhancer ran in a bare forEach with no try/catch anywhere, and
build() inserts its wrapper via `select.parentNode.insertBefore(...)`.
A detached select makes that a null deref, and one throw abandoned the
rest of the pass — every select AFTER it silently stayed native. The
MutationObserver callback had the same exposure for the remaining
mutation records in a batch.

The app config page is the one that can produce a detached select: it
builds its category panels in an async loop, so the observer can see a
node a later render already replaced. That matches "only app config".

Worse than losing the theme: if the throw landed after build() added
.custom-select-native, the select was left opacity:0 / pointer-events:
none behind a button with no listeners — a dropdown that looks right and
does nothing.

Now: detached/unconnected selects are skipped (they get enhanced when
their subtree is attached and the observer fires again), every
enhancement is individually guarded, a failed one is rolled back so it
can never be left invisible, and failures console.warn with the field
name instead of vanishing.

Verified with jsdom against the real file: detached nodes skipped without
throwing, a failure mid-pass leaves later selects working, and the
survivors open and set their value.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../frontend/core/forms/js/custom-select.js   | 48 +++++++++++++++++--
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/containers/libreportal/frontend/core/forms/js/custom-select.js b/containers/libreportal/frontend/core/forms/js/custom-select.js
index c678068..077c5d7 100644
--- a/containers/libreportal/frontend/core/forms/js/custom-select.js
+++ b/containers/libreportal/frontend/core/forms/js/custom-select.js
@@ -314,12 +314,46 @@
     if (el.closest('.custom-select')) return false;
     if (el.multiple) return false;                 // multi-selects need different UX
     if (el.hasAttribute('data-no-enhance')) return false;
+    // Detached nodes can't be enhanced: build() inserts the wrapper via
+    // select.parentNode, which is null here and would throw. This happens for
+    // real — the app config form builds its category panels in an async loop,
+    // so the observer can see a select that a later render already replaced.
+    // Skipping is safe: attaching the subtree fires the observer again, and the
+    // select is enhanced then, connected.
+    if (!el.isConnected || !el.parentNode) return false;
     return ENHANCE_CLASSES.some(c => el.classList.contains(c));
   }
 
+  // One bad select must never cost the others their dropdown. Before this,
+  // enhancement ran in a bare forEach: a single throw aborted the whole pass,
+  // so every select AFTER it silently stayed native — page-wide breakage from
+  // one edge case, with nothing in the console to say so.
+  function enhanceOne(select) {
+    try {
+      new CustomSelect(select);
+    } catch (err) {
+      // Roll back a half-built widget: a select left with .custom-select-native
+      // is invisible (opacity 0, pointer-events none) — worse than un-enhanced.
+      try {
+        select.classList.remove('custom-select-native');
+        select[ENHANCED] = false;
+        const wrapper = select.closest('.custom-select');
+        if (wrapper && wrapper.parentNode) wrapper.parentNode.insertBefore(select, wrapper);
+        if (wrapper) wrapper.remove();
+      } catch (_) { /* best effort — the native select still works */ }
+      console.warn('[custom-select] could not enhance', select.name || select.id || select, err);
+    }
+  }
+
   function enhanceAll(root = document) {
+    // shouldEnhance is inside the guard too: it walks the DOM (closest), which
+    // can itself throw on a node being torn down mid-pass.
     root.querySelectorAll(ENHANCE_SELECTOR).forEach(s => {
-      if (shouldEnhance(s)) new CustomSelect(s);
+      try {
+        if (shouldEnhance(s)) enhanceOne(s);
+      } catch (err) {
+        console.warn('[custom-select] skipped a select', err);
+      }
     });
   }
 
@@ -330,10 +364,14 @@
       for (const m of mutations) {
         for (const node of m.addedNodes) {
           if (node.nodeType !== 1) continue;
-          if (shouldEnhance(node)) {
-            new CustomSelect(node);
-          } else {
-            enhanceAll(node);
+          // Guarded for the same reason as enhanceAll: a throw here would
+          // abandon the remaining mutation records in this batch, so one
+          // awkward node could leave a whole freshly-rendered form native.
+          try {
+            if (shouldEnhance(node)) enhanceOne(node);
+            else enhanceAll(node);
+          } catch (err) {
+            console.warn('[custom-select] observer pass failed', err);
           }
         }
       }