fix(file-viewer): make previewed video seekable and stop it on close - #284
Merged
Conversation
Two bugs in the File Viewer's media player, both reproduced in a real browser against an 18MB mp4 before and after the fix. 1. Closing the preview left the video playing. closeFilePreview() only dropped the overlay's `visible` class, which is display:none and nothing else, so the audio kept going with no visible player to pause. Detaching the element is not a fix either: a detached HTMLMediaElement plays on until it is garbage collected. _stopFilePreviewMedia() now pauses, drops src and load()s every media element (also on re-open, where overwriting innerHTML had the same effect), which additionally aborts the in-flight download. 2. The scrub bar was inert. file-raw read the whole file and answered 200 with no Accept-Ranges, so Chrome reported video.seekable as [0, 0] and silently reverted `currentTime = x`; Safari refuses to start such media at all. Raw bodies are now streamed and range-aware: Accept-Ranges: bytes on every response, 206 + Content-Range for a Range request, 416 for one past EOF, and a malformed spec ignored (200) per RFC 9110. Parsing is pure in src/web/http-range.ts. Measured on tmp/codeman-crt-v5-66s.mp4 (18MB, 66.6s): before seekable [0, 0] seek to 56.6s reverted to 3.9s close: still playing after seekable [0, 66.56] seek to 56.6s landed at 60.2s close: paused, NETWORK_EMPTY Range slices are byte-identical to `dd`, the full-file path is byte-identical to the file, and the SVG octet-stream/attachment hardening and the 50MB cap are unchanged (the cap is still checked before the range). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Two bugs in the File Viewer's media player, both reproduced in a real browser against an 18MB mp4 before and after the fix.
1. Closing the preview left the video playing.
closeFilePreview()only dropped the overlay'svisibleclass. That isdisplay: noneand nothing else, so the video kept playing with its audio audible and no visible player to pause. Detaching the element is not a fix either: a detachedHTMLMediaElementplays on until it is garbage collected._stopFilePreviewMedia()now pauses, dropssrcandload()s every media element in the preview body, which also aborts the in-flight download. It runs on close and on re-open (overwritinginnerHTMLhad exactly the same detach-but-keep-playing effect when switching files).2. The scrub bar was inert.
file-rawread the whole file and answered200with noAccept-Ranges, so Chrome reportedvideo.seekableas[0, 0]and silently revertedcurrentTime = x; Safari refuses to start such media at all. Raw bodies are now streamed and range-aware:Accept-Ranges: byteson every response,206+Content-Rangefor aRangerequest,416for one past EOF, and a malformed spec ignored (200) per RFC 9110. Parsing is pure and unit tested insrc/web/http-range.ts; the attachments/rawroute gets the same treatment.Measured on a 66.6s, 18MB mp4:
seekable[0, 0][0, 66.56]NETWORK_EMPTYRange slices are byte-identical to
dd, the full-file path is byte-identical to the file, and SVG octet-stream/attachment hardening plus the 50MB cap are unchanged (the cap is still checked before the range).Tests:
test/http-range.test.ts(14 parser cases),test/routes/file-routes-range.test.ts(206/416/Accept-Ranges/bounded-read/security-header contract),test/file-preview-media.test.ts(teardown; the 3 behavior cases were confirmed to fail against unfixedpanels-ui.js). Fullnpm run test:cigreen: 5068 passed. Lint, format, typecheck, frontend-syntax, knip all clean.