fix(epub,font): U+FEFF no-break + setFallback assert symmetry#7
Merged
Conversation
日<U+FEFF>本 previously wrapped identically to 日本 — the ZWNBSP no-break intent was silently dropped because the trailing CJK base inherited the run's default CjkBreak join. FEFF (ZWNBSP) produces no token, so its only effect is an intrinsic no-break: Utf8ClusterAssembler now sets nextJoin = Glue across it, so the next CJK base stages with Glue instead of CjkBreak. The parser's Feff dispatch keeps discarding the codepoint and no longer needs to touch the join. Add a host regression test (ParserCluster.FeffIsNoBreakBetweenCjk): 日<FEFF>本 stays glued (Glue) while plain 日本 stays breakable (CjkBreak). Red before the fix, green after.
setFallback asserted only the fallback's glyphMissHandler == nullptr, but resolveGlyph's pointer-identity miss detection runs on the selected primary font too (any of the four, per style) — not just the fallback. Assert every non-null primary symmetrically so the already-documented precondition is structurally enforced on both sides. Harmless today (the UI primaries that receive a fallback are static built-in fonts); this is a footgun guard for a future ring-buffer-backed primary.
The prior commit set nextJoin = Glue for every FEFF, which clobbered a preceding Space (日<space><FEFF>本): the space exists only as the join, so glueing across the FEFF silently swallowed it (and forbade the break). Guard the upgrade on nextJoin == CjkBreak, so FEFF only converts a breakable CJK adjacency into a no-break and leaves a real space — or an already-set Glue — untouched. Add ParserCluster.FeffDoesNotSwallowPrecedingSpace, driven from the post-space state (nextJoin = Space) the parser hands the assembler. Red before the guard, green after; revert-check confirmed non-vacuous.
The guard added in the prior commit makes the FEFF→Glue upgrade conditional (only when the adjacency is breakable, nextJoin == CjkBreak), but the NonCjkKind::Feff doc and the parser's Feff-case comment still read as unconditional. Correct both to state the assembler upgrades only a breakable adjacency and leaves a real Space or existing Glue untouched.
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.
Two small review follow-ups from the Japanese CJK line-breaking PRs (#4–6). One concern per commit.
FIX 1 — U+FEFF no-break regression (commit 1)
日<U+FEFF>本was wrapping identically to日本: the ZWNBSP no-break intent was silently dropped because the trailing CJK base inherited the run's defaultCjkBreakjoin. FEFF produces no token, so its only effect is an intrinsic no-break —Utf8ClusterAssemblernow setsnextJoin = Glueacross it, so the next CJK base stages withGlue. The parser'sFeffdispatch keeps discarding the codepoint and no longer touches the join.Reproduce-first: added host regression test
ParserCluster.FeffIsNoBreakBetweenCjk—日<FEFF>本stays glued (Glue) while plain日本stays breakable (CjkBreak). Red before the fix, green after.FIX 2 — setFallback assert symmetry (commit 2)
setFallbackasserted only the fallback'sglyphMissHandler == nullptr, butresolveGlyph's pointer-identity miss detection runs on the selected primary font too (any of the four, per style). The assert now checks every non-null primary symmetrically, enforcing the already-documented precondition on both sides. Harmless today (UI primaries are static built-ins); a footgun guard for a future ring-buffer-backed primary.Verification
cmake --build build/test && ctest), including the new FEFF regression test (red→green observed) and all font-resolver suites.piobuild + clang-format: left to CI.