From da6c2e2dd930aab8ab3fc303fe5963d359507f8d Mon Sep 17 00:00:00 2001
From: tryptz <216453278+tryptz@users.noreply.github.com>
Date: Sun, 5 Apr 2026 01:25:35 +0000
Subject: [PATCH] fix: address CodeRabbit review issues from PR #502

Android:
- Fix stale wake lock detection by checking isHeld() before reacquiring
- Wrap startForegroundService in try-catch to handle launch exceptions
- Add START/STOP intent actions to prevent stop/start race condition

HTML/Accessibility:
- Convert preset and preamp spans to proper <label> elements
- Add aria-controls to database collapse button
- Sync aria-expanded state when toggling database panel

JavaScript:
- Guard auto-resume with source check to avoid resuming when not playing
- Add _calculateQ() fallback in fast-path EQ filter updates
- Persist graphic EQ gains and preamp to storage from setters
- Include 'legacy' in EQ mode restoration check
- Clear container before rebuilding GEQ bands for idempotency
- Use AbortController for document-level listener cleanup
- Apply gain calculation for non-parametric modes in touch snap
- Validate stored GEQ values are finite numbers

CSS:
- Remove non-standard appearance: slider-vertical property
---
 .../music/AudioPlaybackService.java           |  8 +++++
 .../music/BackgroundAudioPlugin.java          | 18 ++++++----
 index.html                                    |  5 +--
 js/audio-context.js                           |  7 ++--
 js/settings.js                                | 35 +++++++++++++++----
 js/storage.js                                 | 10 ++++--
 styles.css                                    |  1 -
 7 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/android/app/src/main/java/tf/monochrome/music/AudioPlaybackService.java b/android/app/src/main/java/tf/monochrome/music/AudioPlaybackService.java
index 8fbc5c0..e951449 100644
--- a/android/app/src/main/java/tf/monochrome/music/AudioPlaybackService.java
+++ b/android/app/src/main/java/tf/monochrome/music/AudioPlaybackService.java
@@ -33,6 +33,11 @@ public class AudioPlaybackService extends Service {
 
     @Override
     public int onStartCommand(Intent intent, int flags, int startId) {
+        if (intent != null && "STOP".equals(intent.getAction())) {
+            stopSelf();
+            return START_NOT_STICKY;
+        }
+
         Notification notification = buildNotification();
 
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
@@ -95,6 +100,9 @@ public class AudioPlaybackService extends Service {
     }
 
     private void acquireWakeLock() {
+        if (wakeLock != null && !wakeLock.isHeld()) {
+            wakeLock = null;
+        }
         if (wakeLock == null) {
             PowerManager pm = (PowerManager) getSystemService(POWER_SERVICE);
             if (pm != null) {
diff --git a/android/app/src/main/java/tf/monochrome/music/BackgroundAudioPlugin.java b/android/app/src/main/java/tf/monochrome/music/BackgroundAudioPlugin.java
index 6588d6a..afb5d76 100644
--- a/android/app/src/main/java/tf/monochrome/music/BackgroundAudioPlugin.java
+++ b/android/app/src/main/java/tf/monochrome/music/BackgroundAudioPlugin.java
@@ -18,18 +18,24 @@ public class BackgroundAudioPlugin extends Plugin {
 
     @PluginMethod
     public void start(PluginCall call) {
-        Intent intent = new Intent(getContext(), AudioPlaybackService.class);
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-            getContext().startForegroundService(intent);
-        } else {
-            getContext().startService(intent);
+        try {
+            Intent intent = new Intent(getContext(), AudioPlaybackService.class);
+            intent.setAction("START");
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+                getContext().startForegroundService(intent);
+            } else {
+                getContext().startService(intent);
+            }
+            call.resolve();
+        } catch (Exception e) {
+            call.reject("Failed to start audio service: " + e.getMessage(), e);
         }
-        call.resolve();
     }
 
     @PluginMethod
     public void stop(PluginCall call) {
         Intent intent = new Intent(getContext(), AudioPlaybackService.class);
+        intent.setAction("STOP");
         getContext().stopService(intent);
         call.resolve();
     }
diff --git a/index.html b/index.html
index 111c115..f1bb5d7 100644
--- a/index.html
+++ b/index.html
@@ -4259,7 +4259,7 @@
                                     <!-- Legacy 16-Band Graphic EQ (visible in legacy mode) -->
                                     <div class="graphic-eq-section" id="graphic-eq-section" style="display: none">
                                         <div class="graphic-eq-preset-row">
-                                            <span class="graphic-eq-preset-label">Preset</span>
+                                            <label for="legacy-graphic-eq-preset-select" class="graphic-eq-preset-label">Preset</label>
                                             <select
                                                 id="legacy-graphic-eq-preset-select"
                                                 class="graphic-eq-preset-select"
@@ -4288,7 +4288,7 @@
                                         </div>
                                         <div class="graphic-eq-bottom-row">
                                             <div class="graphic-eq-preamp">
-                                                <span class="graphic-eq-preamp-label">Preamp</span>
+                                                <label for="legacy-graphic-eq-preamp-slider" class="graphic-eq-preamp-label">Preamp</label>
                                                 <input
                                                     type="range"
                                                     id="legacy-graphic-eq-preamp-slider"
@@ -4368,6 +4368,7 @@
                                                     id="autoeq-database-collapse"
                                                     aria-label="Collapse database"
                                                     aria-expanded="true"
+                                                    aria-controls="autoeq-database-body"
                                                 >
                                                     <use svg="!lucide/chevron-up.svg" size="18" />
                                                 </button>
diff --git a/js/audio-context.js b/js/audio-context.js
index 5752d93..037c1c1 100644
--- a/js/audio-context.js
+++ b/js/audio-context.js
@@ -353,7 +353,7 @@ class AudioContextManager {
                     console.log(`[AudioContext] State changed to ${this.audioContext.state}, attempting resume`);
                     // Use a short delay to let the system settle before resuming
                     setTimeout(() => {
-                        if (this.audioContext && this.audioContext.state !== 'running') {
+                        if (this.audioContext && this.audioContext.state !== 'running' && this.source) {
                             this.audioContext.resume().catch((e) => {
                                 console.warn('[AudioContext] Auto-resume failed:', e);
                             });
@@ -847,7 +847,7 @@ class AudioContextManager {
                     filter.type = newTypes[i] || 'peaking';
                     filter.frequency.setTargetAtTime(newFrequencies[i], now, 0.005);
                     filter.gain.setTargetAtTime(newGains[i], now, 0.005);
-                    filter.Q.setTargetAtTime(newQs[i], now, 0.005);
+                    filter.Q.setTargetAtTime(newQs[i] > 0 ? newQs[i] : this._calculateQ(i), now, 0.005);
                 });
             } else {
                 // Band count changed — must rebuild
@@ -1056,6 +1056,7 @@ class AudioContextManager {
             const now = this.audioContext.currentTime;
             this.geqFilters[bandIndex].gain.setTargetAtTime(this.geqGains[bandIndex], now, 0.01);
         }
+        equalizerSettings.setGraphicEqGains([...this.geqGains]);
     }
 
     setGraphicEqAllGains(gains) {
@@ -1068,6 +1069,7 @@ class AudioContextManager {
                 this.geqFilters[i].gain.setTargetAtTime(this.geqGains[i], now, 0.01);
             }
         });
+        equalizerSettings.setGraphicEqGains([...this.geqGains]);
     }
 
     setGraphicEqPreamp(db) {
@@ -1077,6 +1079,7 @@ class AudioContextManager {
             const now = this.audioContext.currentTime;
             this.geqPreampNode.gain.setTargetAtTime(gainValue, now, 0.01);
         }
+        equalizerSettings.setGraphicEqPreamp(this.geqPreamp);
     }
 }
 
diff --git a/js/settings.js b/js/settings.js
index 5cd33c1..dc6197a 100644
--- a/js/settings.js
+++ b/js/settings.js
@@ -57,6 +57,8 @@ async function getButterchurnPresets(...args) {
 
 // Module-level state for AutoEQ (persists across re-initializations)
 let _autoeqIndex = [];
+let _graphAbortController = null;
+let _graphResizeObserver = null;
 
 export async function initializeSettings(scrobbler, player, api, ui) {
     // Restore last active settings tab
@@ -1287,6 +1289,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
     // Build 16 vertical slider bands into a container
     const buildGeqBands = (container, idPrefix) => {
         if (!container) return;
+        container.innerHTML = '';
         GEQ_LABELS.forEach((_label, i) => {
             const band = document.createElement('div');
             band.className = 'graphic-eq-band';
@@ -2073,6 +2076,15 @@ export async function initializeSettings(scrobbler, player, api, ui) {
             return { x: e.clientX - rect.left, y: e.clientY - rect.top };
         };
 
+        // Clean up previous document-level listeners and observer on re-initialization
+        if (_graphAbortController) _graphAbortController.abort();
+        _graphAbortController = new AbortController();
+        const graphSignal = _graphAbortController.signal;
+        if (_graphResizeObserver) {
+            _graphResizeObserver.disconnect();
+            _graphResizeObserver = null;
+        }
+
         // Document-level mousemove so dragging continues outside the canvas
         document.addEventListener('mousemove', (e) => {
             if (draggedNode === null) return;
@@ -2116,7 +2128,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
                     graphAnimFrame = null;
                 });
             }
-        });
+        }, { signal: graphSignal });
 
         // Canvas-only mousemove for hover cursor changes (when not dragging)
         autoeqCanvas.addEventListener('mousemove', (e) => {
@@ -2145,7 +2157,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
                 draggedNode = null;
                 autoeqCanvas.style.cursor = hoveredNode >= 0 ? 'grab' : 'crosshair';
             }
-        });
+        }, { signal: graphSignal });
 
         autoeqCanvas.addEventListener('mouseleave', () => {
             // Only reset hover state, NOT drag state (drag continues outside canvas)
@@ -2291,6 +2303,11 @@ export async function initializeSettings(scrobbler, player, api, ui) {
                             if (isParam) {
                                 const newGain = yToDb(coords.y - padTop, h, dbMin, dbMax);
                                 bands[touchNodeIdx].gain = Math.max(-30, Math.min(30, Math.round(newGain * 10) / 10));
+                            } else {
+                                const corrGain = interpolate(bands[touchNodeIdx].freq, autoeqCorrectedCurve || []);
+                                const newDb = yToDb(coords.y - padTop, h, dbMin, dbMax);
+                                const gainDelta = newDb - corrGain;
+                                bands[touchNodeIdx].gain = Math.max(-30, Math.min(30, bands[touchNodeIdx].gain + gainDelta * 0.3));
                             }
                             draggedNode = touchNodeIdx;
                             computeCorrectedCurve();
@@ -2353,7 +2370,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
                 }
                 e.preventDefault();
             },
-            { passive: false }
+            { passive: false, signal: graphSignal }
         );
 
         document.addEventListener('touchend', () => {
@@ -2361,14 +2378,14 @@ export async function initializeSettings(scrobbler, player, api, ui) {
                 draggedNode = null;
                 touchNodeIdx = -1;
             }
-        });
+        }, { signal: graphSignal });
 
         // Resize observer for graph
         if (autoeqGraphWrapper) {
-            const ro = new ResizeObserver(() => {
+            _graphResizeObserver = new ResizeObserver(() => {
                 scheduleDrawAutoEQGraph();
             });
-            ro.observe(autoeqGraphWrapper);
+            _graphResizeObserver.observe(autoeqGraphWrapper);
         }
     }
 
@@ -2529,6 +2546,10 @@ export async function initializeSettings(scrobbler, player, api, ui) {
             if (autoeqDatabaseCollapse) autoeqDatabaseCollapse.classList.toggle('collapsed');
             if (autoeqDatabaseBody)
                 autoeqDatabaseBody.style.display = autoeqDatabaseBody.style.display === 'none' ? '' : 'none';
+            if (autoeqDatabaseCollapse) {
+                const isExpanded = !autoeqDatabaseCollapse.classList.contains('collapsed');
+                autoeqDatabaseCollapse.setAttribute('aria-expanded', String(isExpanded));
+            }
         });
     }
 
@@ -4813,7 +4834,7 @@ export async function initializeSettings(scrobbler, player, api, ui) {
 
     // Restore EQ mode on startup
     const savedEQMode = localStorage.getItem(EQ_MODE_KEY);
-    if (savedEQMode && ['autoeq', 'parametric', 'speaker'].includes(savedEQMode)) {
+    if (savedEQMode && ['autoeq', 'parametric', 'speaker', 'legacy'].includes(savedEQMode)) {
         setEQMode(savedEQMode);
     }
 
diff --git a/js/storage.js b/js/storage.js
index 9f35aff..222fa10 100644
--- a/js/storage.js
+++ b/js/storage.js
@@ -1724,7 +1724,9 @@ export const equalizerSettings = {
             const stored = localStorage.getItem(this.GEQ_GAINS_KEY);
             if (stored) {
                 const parsed = JSON.parse(stored);
-                if (Array.isArray(parsed) && parsed.length === 16) return parsed;
+                if (Array.isArray(parsed) && parsed.length === 16) {
+                    return parsed.map((v) => (Number.isFinite(v) ? v : 0));
+                }
             }
         } catch {
             /* ignore */
@@ -1743,7 +1745,11 @@ export const equalizerSettings = {
     getGraphicEqPreamp() {
         try {
             const val = localStorage.getItem(this.GEQ_PREAMP_KEY);
-            return val !== null ? parseFloat(val) : 0;
+            if (val !== null) {
+                const num = parseFloat(val);
+                return Number.isFinite(num) ? num : 0;
+            }
+            return 0;
         } catch {
             return 0;
         }
diff --git a/styles.css b/styles.css
index 2c4ca96..c8cb658 100644
--- a/styles.css
+++ b/styles.css
@@ -7794,7 +7794,6 @@ body:has(#side-panel.active) #close-fullscreen-cover-btn {
 .graphic-eq-band-slider-wrap input[type='range'] {
     writing-mode: vertical-lr;
     direction: rtl;
-    appearance: slider-vertical;
     width: 28px;
     height: 100%;
     accent-color: var(--foreground);