Repository navigation
Fix importing your own transcript dropping the picked file - #112
Open
chiragjangiir wants to merge 1 commit into
Open
chiragjangiir wants to merge 1 commit into
chiragjangiir wants to merge 1 commit into
Conversation
Both "import your own transcript" inputs captured `e.target.files` and then cleared `e.target.value` before reading it. `input.files` is a live [SameObject] FileList, so clearing the input emptied the very list the handler was still holding: `files[0]` came back undefined, the handler treated the pick as a cancel, `pendingTranscript` was never set, and `loadVideo` ran without words — so Whisper transcribed a video the user had already captioned. That is what wassgha#105 reports, and clearing the input is only meant to allow re-picking the same file. Read the chosen file before resetting the input, in one helper both inputs now share. The in-editor "replace captions" handler takes the File rather than a FileList. Verified in Chromium 130 that clearing `value` does empty an already-captured FileList, and regression-tested against a file input that reproduces that behaviour. Also wires tests/parse-transcript-test.ts into CI: it existed but no script or workflow step ran it. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@chiragjangiir is attempting to deploy a commit to the wassgha's projects Team on Vercel. A member of the Team first needs to authorize it. |
Myoh01
approved these changes
Oct 27, 2026
Myoh01
approved these changes
Oct 27, 2026
|
Sorry, something wrong must've happened, I did not review this, so ignore that. |
This branch has not been deployed
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 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
Importing an SRT, VTT or JSON caption file did nothing: the app silently fell
back to transcribing with Whisper. Both transcript file inputs dropped the file
the user picked, so the "skip Whisper" path was never taken.
Fixes #105.
Root cause
Both inputs did this:
input.filesis a live[SameObject]FileList, so clearing the input emptiesthe very object the handler is still holding.
filecame backundefined, thehandler treated the pick as a cancel,
pendingTranscriptwas never set, andloadVideoran without words — soskipTranscriptionstayedfalseand Whispertranscribed a video the user had already captioned.
Measured in Chromium 130, and then in the app itself with a temporary probe in
the old handler:
Clearing the input is only there to allow re-picking the same file, so the fix is
to read the file first and clear after. The media input in
Editor.tsxalreadydid it in that order, which is why uploading video still worked.
Implementation
lib/parseTranscript.ts— newtakeSelectedTranscriptFile(input): readsinput.files[0], then resets the input, with the ordering documented.components/ImportTranscriptOption.tsx— the upload-screen "import your owntranscript" input uses it. The rest of the handler (progress, error, menu and
cancel-timer bookkeeping) is unchanged.
components/TranscriptPanel.tsx— the in-editor "replace captions" input usesit, and
handleImportTranscriptnow takes theFileinstead of aFileList,so no cast is needed to keep the async handler working.
tests/transcript-file-input-test.ts— new regression tests.package.json+.github/workflows/ci.yml— atest:transcriptscript and aCI step. This also wires up
tests/parse-transcript-test.ts, which existed butwas run by no script or workflow.
Nothing changes for users who do not import a transcript:
loadVideostill setsskipTranscription: falsewhen it is called without words.Testing
Each command below was run; results are the actual output.
The new test file covers: the harness reproducing the clear-then-read bug (a
control block, so the suite fails loudly if the browser behaviour it models ever
changes), SRT/VTT/JSON parsing through the helper, an empty transcript throwing
instead of importing nothing, cancel yielding
null, re-picking the same file,an unsupported file still being handed to the UI so it can report it, and the
end-to-end consequence —
loadVideowith words setsskipTranscription: truewhile
loadVideowithout words leaves itfalse.Verified by hand in the running app (
npm run dev, Chromium), same injectionagainst old and new code:
mycaptions.srtmycaptions.srt; "Will use mycaptions.srt"With the fix, picking an SRT and then a 20s WAV opened the editor with the
imported words in the transcript panel — no transcription ran — and the
in-editor "Import" (replace captions) control replaced the transcript with a
different file.
Limitations
DOM/component test setup and the tests are standalone
tsxscripts. Theregression test exercises the shared helper and a file-input double whose
behaviour was measured in Chromium 130; the component wiring was confirmed by
hand in the running app, as described above.
handler and are covered by the parser and helper tests.
npm run test:ffmpegreport: the native render tests skip unlessnpm run fetch:ffmpeghas run, and they were skipped here.