Fix three bugs: timecode whitespace regression, culture parsing, F10 shortcut override#12545
Merged
Conversation
1. TimeCode.ParseToMilliseconds: a whitespace-only sub-second field (e.g. "0:0:1: ") returned 0 instead of 1000 after the round-2 span rewrite. The legacy PadRight(3,'0') turned " " into " 00" (leading whitespace, which int.Parse tolerates -> 0), but the hand-rolled "trailing whitespace fails" shortcut rejected it outright. Restore exact PadRight semantics by padding into a stack buffer. Reachable from malformed time codes in niche importers. 2. Culture-unsafe float/double parsing in 38 subtitle-format files: bare double.Parse/float.Parse/Convert.ToDouble without InvariantCulture would misread decimal values (e.g. a "1.5" frame/timebase as 15) on a comma-decimal locale like German, corrupting imported timings. Add CultureInfo.InvariantCulture to every single-argument numeric parse in SubtitleFormats. Integer-valued fields are unaffected; the genuinely decimal ones (Final Cut Pro durations, JacoSub) are fixed. 3. F10 menu-focus shortcut couldn't be overridden (#12504): F10 always activated the menu bar, ignoring a user-assigned F10 action. Add IShortcutManager.HasSingleKeyShortcut and skip the built-in menu toggle when the user has bound F10; the menu stays reachable via Alt. Same guard in the main window and the binary-edit window. Verified: repro before/after for all three; new TimeCode parse tests (incl. the whitespace-fraction regression and the digit+space fail case) and a de-DE JacoSub load; all 492 libse + 233 seconv tests pass; full solution builds. Co-Authored-By: Claude Fable 5 <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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Three verified bugs found by an adversarial hunt (each reproduced before/after).
1.
TimeCode.ParseToMilliseconds— whitespace-only fraction regressionIntroduced by this morning's round-2 span rewrite. A whitespace-only sub-second field returned 0 instead of the legacy value:
The old
parts[3].PadRight(3,'0')turned" "into" 00", whichint.Parsereads as 0 (leading whitespace tolerated), so the seconds still counted. The rewrite's hand-rolled "trailing whitespace fails" shortcut rejected it outright. Fixed by padding into a stack buffer — exactPadRightsemantics, so" "→ 0 but"5 "still fails (embedded whitespace). Reachable from malformed time codes in niche importers; low severity but a real behavior change vs. the "byte-identical" commit claim.2. Culture-unsafe numeric parsing in 38 format files
double.Parse/float.Parse/Convert.ToDoublewithoutInvariantCulture. On a comma-decimal locale (German, French, …) a decimal value like a"1.5"Final Cut Pro duration parses as 15 — corrupting imported timings. SweptCultureInfo.InvariantCultureinto every single-argument numeric parse acrossSubtitleFormats/. Integer-valued fields (most FCP timebase/rational tokens, MPlayer2 deciseconds) are unchanged; the genuinely decimal ones are fixed. Verified ade-DEJacoSub load parses correctly.The top-tier formats (SubRip, ASSA, WebVTT, TimedText10, EBU STL, PAC, MicroDVD, SAMI) were already invariant — this hardens the long tail.
3. F10 menu-focus shortcut can't be overridden — #12504
F10 (added in #11745 to activate the menu bar for keyboard/screen-reader access) unconditionally swallowed the key, so a user-assigned F10 action never fired. Added
IShortcutManager.HasSingleKeyShortcut(keyName); the built-in menu toggle now yields when the user has bound F10 to something. The menu bar stays reachable via Alt. Same guard in the main window and the binary-edit (image-sub) window.Verified
TimeCodeParseTest(7 cases incl. the whitespace-fraction regression and the digit+space fail case).🤖 Generated with Claude Code