Fix Crossfade randomly muting music until manual unmute#4307
Fix Crossfade randomly muting music until manual unmute#4307Orionicelion wants to merge 1 commit intopear-devs:masterfrom
Conversation
| if ( | ||
| video.currentTime >= | ||
| video.duration - (this.config?.secondsBeforeEnd ?? 0) && | ||
| video.duration - (this.config?.secondsBeforeEnd ?? 0) && |
There was a problem hiding this comment.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Insert ··
| video.duration - (this.config?.secondsBeforeEnd ?? 0) && | |
| video.duration - (this.config?.secondsBeforeEnd ?? 0) && |
|
|
||
| const crossfade = (cb: () => void) => { | ||
| if (!isReadyToCrossfade()) { | ||
| ensureVideoVolume() |
There was a problem hiding this comment.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Insert ;
| ensureVideoVolume() | |
| ensureVideoVolume(); |
|
|
||
| const crossfade = (cb: () => void) => { | ||
| if (!isReadyToCrossfade()) { | ||
| ensureVideoVolume() |
There was a problem hiding this comment.
🚫 [eslint] <stylistic/semi> reported by reviewdog 🐶
Missing semicolon.
| ensureVideoVolume() | |
| ensureVideoVolume(); |
There was a problem hiding this comment.
Pull request overview
This PR aims to fix a recurring issue in the crossfade plugin where music can end up muted (video volume stuck at 0) after transitions, requiring manual unmute.
Changes:
- Initialize
waitForTransitionand introduceoriginalVolumetracking to restore volume after crossfade-related volume changes. - Add
ensureVideoVolume()and invoke it in several transition/navigation paths to recover from unintended mute states. - Adjust crossfade/sync logic to use
originalVolumewhen fading back in and to trigger a fade-in when the video is already playing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| video.addEventListener('seeking', onSeeking); | ||
| video.addEventListener('pause', onPause); | ||
| video.addEventListener('play', onPlay); |
There was a problem hiding this comment.
syncVideoWithTransitionAudio adds seeking/pause/play listeners every time it’s called, but they are never removed. Since createAudioForCrossfade calls this on each navigation, listeners can accumulate on the same <video> element, leading to duplicated transitionAudio.play/pause/seek calls and repeated fades. Consider removing prior listeners (store handler refs in outer scope) or using an AbortController/signal to clean them up when unloading/creating a new transitionAudio.
| // Fade in | ||
| const videoVolume = video.volume; | ||
| const videoVolume = originalVolume || video.volume || 1; | ||
| video.volume = 0; | ||
| videoFader.fadeTo(videoVolume); |
There was a problem hiding this comment.
The volume restoration/fade target uses originalVolume || video.volume || 1. Because 0 is falsy, a legitimate 0 volume will fall through to 1, which can cause an unwanted jump/unmute. Prefer an explicit > 0 check or nullish coalescing (??) with a separately tracked “last non-zero volume”.
| const ensureVideoVolume = () => { | ||
| const video = document.querySelector('video'); | ||
| if (video && video.volume === 0 && originalVolume > 0) { | ||
| video.volume = originalVolume; | ||
| } | ||
| }; |
There was a problem hiding this comment.
This change introduces non-trivial volume state recovery behavior (tracking/restoring previous volume across navigations). There are Playwright tests in the repo (including plugin-level unit tests), but this plugin has no coverage for crossfade/volume handling. Adding a regression test (unit test for extracted helper or an e2e test that reproduces the “volume stuck at 0 after transition” case) would help prevent future regressions.
| const ensureVideoVolume = () => { | ||
| const video = document.querySelector('video'); | ||
| if (video && video.volume === 0 && originalVolume > 0) { | ||
| video.volume = originalVolume; | ||
| } | ||
| }; |
There was a problem hiding this comment.
ensureVideoVolume will restore the video volume to originalVolume whenever video.volume === 0. Since originalVolume defaults to 1 and is updated whenever the user volume is > 0, this can unintentionally unmute users who explicitly set volume to 0 (or start muted), and can also override intentional “mute by volume=0” behavior. Consider tracking whether the plugin set the volume to 0 (e.g., a pluginMuted/didFadeToZero flag) and only restoring in that case, and initialize originalVolume from the current video element when available instead of a hard-coded 1.
| if (!isReadyToCrossfade()) { | ||
| ensureVideoVolume() | ||
| cb(); |
There was a problem hiding this comment.
Missing semicolon after ensureVideoVolume(); this file otherwise uses semicolons consistently and this will likely fail lint/format checks.
Fixes multiple open issues on the crossfade plugin muting after a transition