Fix freeze caused by progress bar firing onDone during asset transitions

[3rob3]

The progress bar's onChange handler was unconditionally calling play()
whenever the duration changed, causing onDone to fire multiple times
in rapid succession during asset transitions. This created a race
condition that caused a freeze.

Fixed by only calling play() in onChange when the progress bar is
already in a playing state. Also replaced the boolean
isHandlingAssetTransition guard with a promise-based transitionLock
to safely serialize any concurrent handleDone calls.

Summary by CodeRabbit

• Bug Fixes
  • Improved asset transition reliability by ensuring all transition operations execute in the correct sequence without interruption from concurrent requests.
  • Enhanced cleanup of video elements when components are destroyed to prevent playback issues and memory leaks.
  • Fixed event handlers from invoking after component destruction to prevent unexpected behavior and errors.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 12b08e5c-fdc2-407f-9e1d-f42bb72418e9

📥 Commits

Reviewing files that changed from the base of the PR and between 438d59b and 02a8aad.

📒 Files selected for processing (1)

• immichFrame.Web/src/lib/components/elements/asset.svelte

---

📝 Walkthrough

Walkthrough

These changes improve transition handling and component lifecycle management. The home-page component replaces a boolean guard with a serialized Promise queue to properly order concurrent transition requests. The asset component adds proper cleanup on destroy, clears video sources, and guards event handlers to prevent post-destruction invocations.

Changes

Cohort / File(s)	Summary
Transition Queue Serialization immichFrame.Web/src/lib/components/home-page/home-page.svelte	Replaced isHandlingAssetTransition boolean with transitionLock: Promise<void> to queue async transitions sequentially rather than skipping concurrent requests. Each handleDone call chains onto the prior lock, executes the full transition sequence, and handles errors via logging instead of rethrowing.
Component Lifecycle Cleanup immichFrame.Web/src/lib/components/elements/asset.svelte	Added onDestroy handler that marks component as destroyed, pauses video playback, clears video source entirely, and removes the src attribute. Video event handlers (onwaiting, onplaying) now guard against post-destroy invocations. Removed previous onerror handler.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 With transitions now queued in orderly line,
And videos cleaned when their curtains must shine,
No ghost handlers haunt what has passed into night—
Each asset says goodbye with a clear, tidy flight. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the main fix: preventing a freeze caused by the progress bar firing onDone during asset transitions, which aligns with the core changes to transition handling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev.3rob3.VideoFreezeFix

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

immichFrame.Web/src/lib/components/home-page/home-page.svelte (1)

152-170: Serialized transition queue looks good; a couple of minor observations.

Replacing the boolean guard with a promise chain correctly serializes handleDone invocations and prevents the reentrancy that caused the freeze. The .catch on the inner promise also ensures a single failed transition doesn't poison the chain for subsequent calls.

Two small, optional considerations:

1. progressBar.restart(false) (line 158) and progressBar.play() (line 164) are not awaited. Not awaiting play() is intentional and correct (it resolves only when the tween reaches 1, i.e. after the full interval). But not awaiting restart(false) means progress.set(0) races with the downstream progressBar.play() → progress.set(1) on the (newly re-assigned) tween from the duration change. In practice setDuration returns 0 ms when to === 0, so set(0) settles synchronously on the next microtask, but awaiting it would make the ordering explicit and robust to future changes in setDuration.

2. The chain transitionLock = transitionLock.then(...).catch(...) is unbounded in principle; each resolved predecessor becomes eligible for GC, so this is fine in practice, just worth being aware of if transitions are ever triggered at very high frequency.

Optional hardening

 		transitionLock = transitionLock
 			.then(async () => {
 				userPaused = false;
-				progressBar.restart(false);
+				await progressBar.restart(false);
 				$instantTransition = instant;
 				if (previous) await getPreviousAssets();
 				else await getNextAssets();
 				await tick();
 				await assetComponent?.play?.();
 				progressBar.play();
 			})

🤖 Prompt for AI Agents

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

In `@immichFrame.Web/src/lib/components/home-page/home-page.svelte` around lines
152 - 170, The transition handler handleDone should await
progressBar.restart(false) before calling progressBar.play() to ensure restart's
progress.set(0) completes before play's progress.set(1) runs; update the async
callback inside transitionLock.then(...) in handleDone to await
progressBar.restart(false) (maintaining the existing .catch(...) on the chain)
so ordering is explicit and robust to future changes in setDuration.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@immichFrame.Web/src/lib/components/home-page/home-page.svelte`:
- Around line 152-170: The transition handler handleDone should await
progressBar.restart(false) before calling progressBar.play() to ensure restart's
progress.set(0) completes before play's progress.set(1) runs; update the async
callback inside transitionLock.then(...) in handleDone to await
progressBar.restart(false) (maintaining the existing .catch(...) on the chain)
so ordering is explicit and robust to future changes in setDuration.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 83dbd6b6-18bd-4894-9c07-091b51432fb4

📥 Commits

Reviewing files that changed from the base of the PR and between 265a890 and 438d59b.

📒 Files selected for processing (2)

• immichFrame.Web/src/lib/components/elements/progress-bar.svelte
• immichFrame.Web/src/lib/components/home-page/home-page.svelte

[3rob3]

Closes #601