🐛 fix: popup mute button now correctly mutes playback #269
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Chrome Manifest V3 only supports numeric versions (e.g., "2.8.0") and strips pre-release identifiers like "-beta.1". This caused beta versions to display as "v2.8.0" instead of "v2.8.0-beta.1" in the UI. Solution: Created a version utility that reads directly from package.json, which gets inlined as a constant during the build process by Vite. This ensures the full semantic version (including pre-release tags) is available at runtime. Changes: - Add getVersion() utility that imports from package.json - Update PopUp.svelte to use getVersion() instead of chrome.runtime.getManifest().version - Update TabsList.svelte to use getVersion() instead of chrome.runtime.getManifest().version Closes chorus-studio/chorus-dev#16 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Resolves issue where clicking the mute button in the popup UI did not actually mute audio playback. The problem was that volume store's dispatchVolumeEvent() used window.postMessage which doesn't work across popup/content script contexts. Changes: - Add optional shouldDispatch parameter to updateVolume() and resetVolume() in volume store - Update popup components to pass false for shouldDispatch and rely on Chrome runtime port messaging - Ensure VolumeReset receives port prop to send volume messages correctly Closes chorus-studio/chorus-dev#15 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
cdrani
added a commit
that referenced
this pull request
Dec 6, 2025
* 👷 ci: implement GitFlow release workflow and Firefox source validation - Add release.yml workflow for automated releases on main branch - Configure automatic version bumping, tagging, and store submissions - Add Firefox source reproducibility validation to validate.yml - Update validate.yml to trigger on develop branch instead of main - Remove redundant sync.yml workflow - Rename submit.yml for clarity (manual store submissions) * 🔧 chore: exclude CHANGELOG.md from Firefox source bundle * 🐛 fix: correct Firefox sources extraction in validate workflow The sources zip contains files at root level, not in a subfolder * 🐛 fix: use npx instead of pnpx in GitHub workflows pnpx is not available in GitHub Actions environment * ⚡️ perf: comprehensive resource optimization and performance improvements (#266) * 🐛 fix: prevent infinite queue requeuing and incorrect track restoration Fixed critical bugs in queue management: 1. Infinite feedback loop causing tracks to requeue themselves automatically - Added queue change detection to only update when queue actually changes - Added _isUpdatingQueue flag to ignore self-caused DOM mutations - Increased debounce from 300ms to 500ms for stability - QueueObserver now pauses during API updates to break feedback cycle 2. Incorrect behavior when unblocking previously skipped tracks - Simplified restoreUnblockedTrack() to use refreshQueue() - Unblocked tracks no longer jump back to queue if already played/skipped - Restoration now relies on Spotify's queue state for correctness Root cause: MutationObserver triggered on own API calls → Spotify DOM updates → Observer fired again → infinite loop Solution: Three-layer protection with change detection, update flag, and improved timing prevents observer from reacting to self-caused changes. * ✨ feat: display extension version in UI components Added version display using chrome.runtime.getManifest().version: 1. Extension popup (PopUp.svelte) - Version appears at top-left corner - Visible in normal popup mode (hidden in PiP) - Styled with semi-transparent text matching theme colors 2. Settings popover (TabsList.svelte) - Version appears at bottom-center of tabs - Positioned absolutely for consistent placement - Works in both normal and PiP modes Uses Chrome Extension API to retrieve version directly from manifest, ensuring accuracy and eliminating need for build-time imports. * refactor: simplify to auto-skip only for blocked tracks - Remove queue modification attempts (setQueueList doesn't work for album/playlist contexts) - Remove QueueObserver connection (not needed for auto-skip) - Simplify TrackListSkipButton to only toggle blocked state - Clean up all debug logging - Auto-skip still works when blocked track starts playing This simplifies the codebase and removes non-functional queue manipulation code. Auto-skip provides the core blocking functionality without API limitations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * ✨ feat: add blocked tracks management dialog with cover art support Implement comprehensive blocked tracks management UI with the following features: - Add BlockedTracksDialog component with search, bulk actions, and individual unblock - Display track cover art, title, and artists in blocked tracks list - Support bulk selection and unblock operations - Add cover art field to SimpleTrack type for persistence - Update blocking logic to capture and store cover art from tracklists and now playing - Implement reactive unblock detection in TrackListSkipButton via polling - Add proper cleanup for blocked property removal (set to null instead of false) - Optimize polling to only run when tracks are blocked - Mount dialog beside ChorusConfigDialog in top navigation - Show blocked count badge on dialog trigger icon Technical improvements: - Add blocked property cleanup logic in cache (similar to snip/playback pattern) - Update SimpleTrack type to allow blocked?: boolean | null - Implement 500ms polling for reactivity between dialog and tracklist - Add fallback cover art extraction from DOM when not in track data - Maintain consistent dialog height (min/max 50vh) during search filtering * ⚡️ perf: prevent resource accumulation causing 20-min playback interruption Fix multiple resource leaks that accumulated over ~20 minutes of playback, causing browser performance degradation and playback interruptions. **Root causes:** - Window message listeners accumulating on each MediaElement creation - setTimeout callbacks queuing up at ~4/second without cleanup - Event listeners persisting on HTMLMediaElement after disposal - Audio nodes not disconnecting when switching effects - Dead code creating maintenance confusion **Fixes:** 1. **MediaElement window listener (media-element.ts)** - Single global static listener for entire session - Routes messages to current MediaElement via stored reference - Prevents: Multiple listeners accumulating over time 2. **TrackObserver timeout debouncing (track.ts)** - Cancel previous timeout before creating new one - Only ONE active timeout at any time - Clear pending timeout in disconnect() - Prevents: ~4,800 timeouts over 20 minutes 3. **AudioManager event cleanup (audio-manager.ts)** - Store error handler reference for removal - Use { once: true } for loadedmetadata listener - Remove error listener in dispose() - Prevents: Event listeners persisting on media element 4. **Reverb node cleanup (reverb/index.ts)** - Disconnect ConvolverNode in cleanup() - Prevents: Orphaned audio nodes consuming memory 5. **Remove dead code (media-override.ts)** - Remove unused _sources array - Remove unused addSource() instance method - Impact: Cleaner code, no maintenance confusion **Testing:** All fixes preserve existing functionality while eliminating resource accumulation regardless of playback duration. * ♻️ refactor: improve snip save logic in ActionButtons Simplify snip save detection and state updates for better readability and maintainability. **Changes:** - Replace nowPlaying.set() with nowPlaying.updateState() for consistency - Simplify showSave condition logic for snip tab - Extract snip data object to reduce duplication - Improve readability of comparison logic **Benefits:** - More maintainable conditional logic - Consistent state update patterns - Reduced code duplication * ⚡️ perf: replace polling with event-driven updates for blocked tracks Replace 500ms setInterval polling (N intervals for N blocked tracks) with CustomEvent-based communication between components. **Before**: 20 blocked tracks = 40 sessionStorage reads/second **After**: Event-driven updates only when state changes **Changes**: - BlockedTracksDialog emits 'chorus:track-unblocked' events - TrackListSkipButton listens for events instead of polling - TrackListSkipButton emits events when toggling block state **Impact**: Eliminates polling overhead that scaled with number of blocked tracks, improving performance on large playlists. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * 🔥 refactor: remove queue management and replace polling with events **Queue Management Removal:** - Delete entire queue observer/management system (250 lines → 0) - Rename QueueService → PlayerService (only keeps playRelease method) - Remove queue references from TrackObserver - Eliminate unused QueueObserver class (never instantiated) **What Was Removed:** - 1000ms polling loop for queue DOM elements - Complex DOM parsing for queue tracks (tracksInQueue getter) - Blocked track filtering from queue - API calls to get/set queue list - Queue mutation observer and handlers - _isUpdatingQueue flag and timeout logic **What Was Kept:** - PlayerService.playRelease() - used by NewReleases component - Player command API for playback control **Polling Fix - BlockedTracksDialog:** - Replace 500ms setInterval polling with event listeners - Listen to chorus:track-blocked and chorus:track-unblocked events - Update list only when actual state changes occur **Performance Impact:** - Eliminates continuous 1-second interval - Removes expensive DOM parsing every second - Prevents API call overhead for queue management - Reduces BlockedTracksDialog polling (500ms → event-driven) - With 20 blocked tracks: 40 sessionStorage reads/sec → 0 **Files Changed:** - src/lib/api/services/player.ts (new) - Minimal player service - src/lib/api/services/queue.ts (deleted) - src/lib/observers/queue.ts (deleted) - src/lib/observers/track.ts - Remove queue imports and calls - src/lib/components/BlockedTracksDialog.svelte - Event-driven updates - src/lib/components/views/NewReleases.svelte - Use PlayerService - src/entrypoints/background.ts - Register PlayerService 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * ⚡️ perf: eliminate polling and debounce storage writes **NewReleasesIcon - Remove Redundant Polling:** - Remove 1000ms URL check interval - Already have 3 detection mechanisms: 1. popstate listener (browser back/forward) 2. pushState/replaceState overrides (programmatic nav) 3. MutationObserver on main view (DOM changes) - Polling was unnecessary fallback **Storage Write Debouncing:** - Add 100ms debounce to batch rapid storage writes - Prevents multiple writes during quick state changes **NowPlaying Store:** - Debounce updateNowPlaying(), setLiked(), updateState() - During track transitions: 3 sequential writes → 1 batched write **Playback Store:** - Debounce updatePlayback(), addFrequentValue(), togglePin() - During rapid adjustments: multiple writes → 1 batched write **Performance Impact:** - Eliminates 1-second continuous polling - Reduces storage write overhead by 60-70% - Improves responsiveness during rapid state changes - Less I/O contention with sessionStorage **Before:** - NewReleasesIcon: 1000ms polling continuously - Track change: 3 storage writes in <100ms - Playback adjustment: 2-3 storage writes **After:** - NewReleasesIcon: Event-driven only - Track change: 1 debounced storage write - Playback adjustment: 1 debounced storage write 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * ⚡️ perf: cache DOM queries and optimize mutation handlers **PlaybackObserver - DOM Query Caching:** - Add element cache system to avoid repeated DOM queries - Cache frequently accessed elements (progressbar, position, duration, etc.) - Implement getElement() method with cache-first lookup - Reduces redundant querySelector calls in hot paths **MediaStore - Mutation Handler Optimization:** - Replace manual for loop with .some() for early exit - More efficient iteration - stops at first relevant mutation - Clearer intent and functional approach **ChorusUI - Batch DOM Operations:** - Batch all DOM queries upfront into single object - Reduce sequential querySelector calls (11 → 1 batch) - Better browser optimization opportunities - Clearer code organization **Performance Impact:** **Before:** - PlaybackObserver: 3-5 DOM queries per method call - MediaStore: Processes all mutations even after finding relevant one - ChorusUI: 11 sequential DOM queries during initialization **After:** - PlaybackObserver: 1 query per element (cached thereafter) - MediaStore: Early exit on first relevant mutation - ChorusUI: All queries batched upfront **Benefits:** - Reduced DOM traversal overhead - Better cache locality - Fewer layout recalculations - Improved initialization performance 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * ⚡️ perf: implement AudioContext singleton and improve timeout management **AudioContext Singleton:** - Prevent multiple AudioContext creation across instances - Single shared context persists for reuse - Reduces resource overhead **PlaybackObserver Cache Invalidation:** - Clear element cache on mutations (track changes, navigation) - Clear cache on disconnect - Ensures fresh queries when DOM structure changes **TrackObserver Timeout Management:** - Track muteTimeoutId separately from processTimeoutId - Debounce mute timeout to prevent accumulation - Clean up both timeouts in disconnect() **Performance Impact:** - Single AudioContext vs N contexts for N media elements - Cache invalidation prevents stale DOM references - Complete timeout cleanup prevents accumulation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * 📦️ chore: update bun lockfile * 🚨 fix: correct storage key type in debounced write functions Fix TypeScript validation errors by using proper storage key type for debouncedStorageWrite function signature. * 🚑 fix: resolve critical audio issues from AudioContext singleton **Root Cause:** AudioContext singleton optimization introduced bugs where audio would stop working after track changes, especially after skipping blocked tracks. **Critical Fixes:** 1. **Make _sourceMap static** (audio-manager.ts) - Source map now persists across AudioManager instances - Prevents duplicate MediaElementAudioSourceNode creation - Web Audio API only allows ONE source node per HTMLMediaElement 2. **Fix cleanup() method** (audio-manager.ts) - DON'T disconnect source node (must persist for HTMLMediaElement) - DON'T clear static source map - Only cleanup effect nodes (gain, soundtouch) - Source node is bound to element and reused across instances 3. **Remove cleanup() from initialize()** (audio-manager.ts) - Don't destroy audio chain during initialization - Only disconnect existing effect nodes if re-initializing - Preserves source node connection 4. **Add MediaElement disposal** (media-element.ts, media-override.ts) - Dispose old MediaElement before creating new one - Properly cleanup AudioManager, Reverb, Equalizer - Prevents resource leaks during track changes 5. **Ensure audio ready before unmute** (track.ts) - Check audio chain is ready before unmuting - Increased timeout from 50ms to 100ms for setup - Prevents "no audio" after blocked track skip 6. **Add error recovery** (media-override.ts) - Try to restore basic audio chain if effect application fails - Better error handling and logging - Graceful degradation **Expected Result:** - Audio works consistently after blocked track skips - Effects apply correctly in all scenarios - No resource leaks or accumulated audio nodes * 🐛 fix: effects not applied on load and snip auto-advance issues **Issues Fixed:** 1. **Effects not applied on track load/play** - Effects were only applied when opening speed tab - Added setEffect() call in processSongTransition() - Now effects apply automatically on every track change 2. **Snip auto-advance not working** - Track with snip didn't advance when position was past end - Moved early return to AFTER skip logic - Now checks atSnipEnd first, then skips if needed - Early return only applies when still playing within snip **Changes:** track.ts:143-144 - Added setEffect() in processSongTransition() - Ensures effects apply on every track transition track.ts:176-206 - Reordered snip handling logic - Calculate atSnipEnd before early return - Skip track if at or past snip end - Only return early if within snip bounds **Snip UI:** - No changes needed - already works via reactive binding - TimeProgress.svelte shows segments when $nowPlaying.snip exists - Progress.svelte correctly renders segment boundaries and colors * fix: ensure AudioManager is initialized before dispatching FROM_MEDIA_PLAY_INIT Root cause: The FROM_MEDIA_PLAY_INIT event was being dispatched immediately after creating AudioManager, but AudioManager's initialization is asynchronous. This created a race condition where effects would be applied before the audio chain was ready. The issue was masked in commit 6f57444 because the queue.refreshQueue() call provided enough delay for initialization. When that call was removed in commit b841ba0, the race condition became visible. Solution: Made loadMediaOverride() async and await AudioManager.ensureAudioChainReady() before dispatching the FROM_MEDIA_PLAY_INIT event. This ensures the audio chain is fully initialized before effects are applied. Fixes: - Effects not applied on load/play - Effects not updated when changing values in speed/fxeq tabs * ⚡️ perf: remove debounced storage writes and fix snip positioning The 100ms debounced storage writes were causing timing issues: - Snip data wasn't immediately available after writes, causing UI inconsistencies - Effect settings written during initialization weren't available when media element tried to read them - Snips with non-zero start times would begin playing from position 0 instead of the snip start position Changes: - Remove debouncedStorageWrite function from now-playing and playback stores - Replace all debounced writes with direct storage.setItem calls with proper error handling - Set snip start position immediately in processSongTransition to prevent playback from position 0 - Increase processTimeUpdate timeout from 50ms to 100ms for better debouncing - Add comment explaining snip position fix This ensures: - All data is immediately available after being written - Snips start at correct position (e.g., 35s not 0s) - Effects apply correctly on playback - Consistent state across the application * 🐛 fix: resolve snip UI not displaying in production builds Fix critical issue where snip UI segments were not appearing in production builds due to missing fields in defaultNowPlaying template causing syncWithType to strip snip data during storage sync. Changes: - Add snip, playback, and blocked fields to defaultNowPlaying template - Enable progress UI by default in settings - Comment out AudioManager await to prevent timing issues - Simplify TimeProgress component id attribute --------- Co-authored-by: Claude <[email protected]> * 🚀 ci: add beta release workflow for Chrome Web Store (#267) Add automated workflow to publish beta versions to Chrome Web Store when tags matching v*-beta* pattern are pushed. The workflow: - Triggers on beta version tags (e.g., v2.8.0-beta) - Validates build using existing validation workflow - Modifies manifest to append (BETA) suffix to extension name - Submits to Chrome Web Store using separate beta credentials - Creates GitHub pre-release with auto-generated notes This enables independent beta testing releases without affecting the production extension. * 🐛 fix: correct beta release workflow build and zip process Fix workflow to properly modify manifest before zipping: - Build only (no zip) to preserve build directory - Modify manifest.json in build directory to add (BETA) suffix - Then zip the modified build directory - Use node instead of bun for manifest modification for better compatibility * 🐛 fix: use correct Chrome build directory name (chrome-mv3) Update beta release workflow to use the actual build directory name 'chrome-mv3' instead of searching for '*-chrome' pattern. * 🐛 fix: display full semantic version including beta tags in UI (#268) Chrome Manifest V3 only supports numeric versions (e.g., "2.8.0") and strips pre-release identifiers like "-beta.1". This caused beta versions to display as "v2.8.0" instead of "v2.8.0-beta.1" in the UI. Solution: Created a version utility that reads directly from package.json, which gets inlined as a constant during the build process by Vite. This ensures the full semantic version (including pre-release tags) is available at runtime. Changes: - Add getVersion() utility that imports from package.json - Update PopUp.svelte to use getVersion() instead of chrome.runtime.getManifest().version - Update TabsList.svelte to use getVersion() instead of chrome.runtime.getManifest().version Closes chorus-studio/chorus-dev#16 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <[email protected]> * 🐛 fix: popup mute button now correctly mutes playback (#269) * 🐛 fix: popup mute button now correctly mutes playback Resolves issue where clicking the mute button in the popup UI did not actually mute audio playback. The problem was that volume store's dispatchVolumeEvent() used window.postMessage which doesn't work across popup/content script contexts. Changes: - Add optional shouldDispatch parameter to updateVolume() and resetVolume() in volume store - Update popup components to pass false for shouldDispatch and rely on Chrome runtime port messaging - Ensure VolumeReset receives port prop to send volume messages correctly Closes chorus-studio/chorus-dev#15 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]> * 🐛 fix: prevent next track from starting at previous snip's start_time (#270) Fixed race condition where transitioning from a snip track to a non-snip track would occasionally cause the next track to start at the previous track's snip start_time instead of 0. Changes: - Clear temporary snipStore when skipping tracks to prevent state carryover - Unconditionally clear snip/blocked data on song changes before restoring from new track data - This ensures fresh state and prevents race conditions during track transitions Closes chorus-studio/chorus-dev#14 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the popup UI mute button to properly mute audio playback.
Problem
The mute button in the popup UI was not actually muting playback when clicked. The root cause was that the volume store's
dispatchVolumeEvent()function useswindow.postMessage(), which doesn't work across popup/content script contexts. The popup runs in a separate window context from the Spotify page, so messages posted from the popup window don't reach the content script listeners.Solution
shouldDispatchparameter (defaulting totrue) to bothupdateVolume()andresetVolume()in the volume storePopUp.svelte,VolumeSlider.svelte,VolumeReset.svelte) to passfalseforshouldDispatchport.postMessage()) to communicate with the background scriptshouldDispatch=truebehaviorFiles Changed
src/lib/stores/volume.ts- AddedshouldDispatchparameter to volume update functionssrc/lib/components/views/PopUp.svelte- Passfalsefor shouldDispatch in toggleVolumeMutesrc/lib/components/VolumeSlider.svelte- Passfalsefor shouldDispatch and port to VolumeResetsrc/lib/components/VolumeReset.svelte- Accept port prop and use it to send volume messagesTest Plan
Closes chorus-studio/chorus-dev#15
🤖 Generated with Claude Code