Preserve unparseable TTML/SRT color values instead of dropping them - #148
Open
jason-valenzuela wants to merge 2 commits into
Open
Preserve unparseable TTML/SRT color values instead of dropping them#148jason-valenzuela wants to merge 2 commits into
jason-valenzuela wants to merge 2 commits into
Conversation
jason-valenzuela
marked this pull request as draft
August 20, 2026 18:53
jason-valenzuela
marked this pull request as ready for review
August 20, 2026 20:28
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.
Problem
newColorFromHTMLStringdecoded only 6-digit#RRGGBBand 8 named colors. For any other value it returned an error, and both callers — the TTML reader (ttml.go) and the SRT reader (srt.go) — gate onerr == nil, so the color was silently discarded on read and omitted on write.That dropped values which are valid per the specs:
#RRGGBBAA(8-digit, with alpha) — valid per TTML1 §8.3.2 and the form IMSC1 mandates fortts:color.silver,gray,maroon,transparent) — part of the TTML1 §8.3.2<namedColor>set.rgb()/rgba()functional notation.A
TTML in → TTML out(or SRT) round-trip therefore lost styling the source explicitly set.Fix
Color.rawfield holding the original expression when it can't be decoded into an RGBA triple;HTMLString()returns it verbatim. So#RRGGBBAA,rgb()/rgba(),transparent, and unrecognized names now round-trip losslessly instead of being dropped.silver,gray,maroon,purple,fuchsia,lime,olive,navy,teal,aqua) into RGBA, reusing the existingColor*vars, alongside the 8 already handled.newColorFromHTMLStringcan no longer fail, so its now-deaderrorreturn is removed and the three call sites simplified to a direct assignment. An empty/blank expression yields anilcolor (attribute stays omitted) rather than being flattened to#000000.Decoded colors still normalize to hex on write (e.g.
white→#ffffff,silver→#c0c0c0), which was already the case for the original 8 and is valid output.Test
TestColorHTMLRoundTripcovers#RRGGBBand named-color decoding, verbatim round-trip of#RRGGBBAA/transparent/ an unrecognized name /rgb(), and empty/blank → nil.gofmt,go build ./..., andgo test ./...all pass.Notes
Scope is intentionally minimal:
#RRGGBBAA,rgb()/rgba(), andtransparentare preserved verbatim rather than decoded into RGBA. Decoding#RRGGBBAAspecifically would needColorto distinguish "alpha absent" from "alpha == 0x00" (the struct'sAlpha uint8defaults to 0), which ripples intoSSAString/WebVTTString; that's a larger change best left as a follow-up.