Repository navigation
IMSC 1.1: keep trailing <br/> forced line breaks - #16005
Merged
Merged
Conversation
The reader built each cue as ReadParagraph(...).TrimEnd(), which also removed the newlines a trailing <br/> adds - so "C<br/><br/><br/>" loaded as "C" and the forced breaks were lost on import (SubtitleEdit#15289). Trim only the text behind the last forced break instead: everything before it already ends in a line break and is content, while the tail is still source formatting. A paragraph without a <br/> trims exactly as before, and nested spans are still read raw, so a space inside the last span survives. Part of SubtitleEdit#15289 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
This was referenced Oct 13, 2026
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
TimedTextImsc11dropped trailing forced line breaks on import: a cue written asC<br/><br/><br/>loaded asC. In TTML<br/>is a forced break with layout meaning (often paired with region extent /tts:displayAlign), so the cue that the file describes is not the cue the reader produced. This is the remaining half of #15289 — the extension-routing half was fixed in #15570.Root cause
LoadSubtitlebuilt each cue as:ReadParagraph()maps every<br/>toAppendLine(), so the trailing breaks were present in the string and then the blanketTrimEnd()removed them along with source formatting whitespace.Reproduced on unmodified
mainwith a reader-level test:The writer already serializes text faithfully (
string.Join("<br/>", text.SplitToLines())), so the loss was reader-only: load → save at format level was not lossless.Solution
Track the end of the last forced break while reading a paragraph, and trim only the text behind it:
<br/>breaks now survive;<br/>is trimmed exactly as before (raw.TrimEnd()), so behaviour for ordinary files is unchanged;<span>content is still read raw and trimmed only in the finished paragraph, so a space inside the last span still survives (Italic_Trailing_Space_Preserved).No changes to timing, paragraph start/end values, or the writer.
Testing
Local: Apple Silicon macOS, .NET SDK 10.0.401.
dotnet test tests/libse/LibSETests.csproj --filter "FullyQualifiedName~TimedTextImsc11Test"Trailing_Forced_Line_Breaks_Are_Preserved,LoadSaveLoad_Keeps_Trailing_Forced_Line_Breaks), 7 passed; after fix: 9/9 passeddotnet build SubtitleEdit.sln --no-restoredotnet test SubtitleEdit.sln --no-build(same as.github/workflows/tests.yml)LibSETests2588/2588 passed,SeConvTests586 passed / 2 skipped,LibUiLogicTests1215 passed,UITests7380 passed / 6 failedNew regression tests:
Trailing_Forced_Line_Breaks_Are_Preserved— the reported case (A<br/><br/><br/>BandC<br/><br/><br/>).Trailing_Source_Whitespace_Is_Still_Trimmed— guards the whitespace half of the oldTrimEnd()(inline trailing space, indented paragraph).LoadSaveLoad_Keeps_Trailing_Forced_Line_Breaks— load →ToText→ load is now lossless.git diff --checkis clean; onlysrc/libse/SubtitleFormats/TimedTextImsc11.csandtests/libse/SubtitleFormats/TimedTextImsc11Test.csare changed.Limitations
UITestsfailures are pre-existing on unmodifiedmainon this machine: I re-ran exactly those tests with my changes stashed and the same 6 failed (SyntaxHighlightingTextPresenterCanaryTests×3,RtlSplitRunHitTestTests,AssaTagRtlAvaloniaCanaryTests,TableViewScrollBarTroughPagingTests). They are headless-Avalonia rendering/style canaries unrelated to subtitle formats. CI runs onubuntu-latest; the real CI result is the authority for that suite.SubtitleLineViewModel.ToParagraph()trims trailing whitespace on every save so a trailing Enter typed in the edit box can never reach a saved file (the cross-format invariant from Remove text for HI reports a trailing empty line as a change #13389). Distinguishing a loaded break from a typed one would need state the text box does not have, so I did not touch it. This PR makes the reader (and everything built onSubtitle— headless conversion, batch, plugins, the text shown in the editor) faithful; saving an untouched file from the GUI still applies the existing trimming rule.SubtitleFormatAPI level, which is the project's supported way to test readers.