Skip to content

LT-22351: Preserve default font features in Preview and Word export - #1005

Open
johnml1135 wants to merge 1 commit into
mainfrom
LT-22351
Open

LT-22351: Preserve default font features in Preview and Word export#1005
johnml1135 wants to merge 1 commit into
mainfrom
LT-22351

Conversation

@johnml1135

@johnml1135 johnml1135 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Persisted OpenType and Graphite defaults now flow through LibLCM's
BaseStyleInfo into both Dictionary Preview CSS and Word export. FieldWorks no
longer maintains a second StyleInfo loader for ktptFontVariations.

The main review question is whether removing that compatibility loader leaves
either export path without coverage. The tests use real persisted IStStyle
rules rather than an in-memory test double, and they verify both propagation
and precedence: a style's own features override writing-system defaults while
the font family can still fall back normally. The branch uses the published
SIL.LCModel 11.0.0-beta0178 package, which contains the authoritative load
path from sillsdev/liblcm#388.

Where to look

  • StyleInfo.cs removes the FieldWorks-only compatibility loader.
  • CssGeneratorTests exercises persisted defaults and CSS precedence.
  • LcmWordGeneratorTests exercises the equivalent Word export behavior.
  • The archived design records why LibLCM is now the authoritative load path.

Deliberately not here

  • No replacement FieldWorks fallback or duplicate font-feature parser.
  • No .NET 10 migration; LibLCM #394 repaired the current .NET 8 CI instead.

Stack and verification

LibLCM #388 and #394 are merged, and beta0178 is published on NuGet.org. The
FieldWorks Debug build passes with 0 warnings and 0 errors. The four focused
xWorks tests covering persisted defaults and precedence pass against beta0178.

LT-22351


Reading this a year from now -- start here

The load-path decision remains in the archived design because it constrains
future style handling. Temporary investigation and proposal narration was
removed from this branch; the dependency sequence is recorded here because it
is specific to landing this pull request.

Decisions and evidence
  • LibLCM owns deserialization of default ktptFontVariations; FieldWorks owns
    consumption in Preview CSS and Word export.
  • Regression tests construct a real style with persisted rules, matching the
    production LcmStyleSheet path that the old test doubles bypassed.
  • Coverage includes the sibling precedence case so the fix cannot silently
    replace a style's own features with writing-system defaults.
  • Focused tests passed against the published beta0178 package containing
    LibLCM Fix LT-21907: Replace Stem Name with Stem Allomorph Label in UX #388.

This change is Reviewable

@github-actions

This comment has been minimized.

@johnml1135

Copy link
Copy Markdown
Contributor Author

Note: CI on this PR is expected to be red until sillsdev/liblcm#388 is released and Build/SilVersions.props is bumped. The committed SilVersions.props still pins upstream 11.0.0-beta0173, which lacks the liblcm fix, so both the pre-existing gate test (SaveToDB_DefaultFontFeatures_RoundTripsThroughRules) and the new CssGenerator regression test in this PR will fail against that pin in CI. This is the expected pre-release state, not a broken branch -- see the DO NOT MERGE note above.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

NUnit Tests

    1 files  ±0      1 suites  ±0   10m 58s ⏱️ -38s
5 765 tests +4  5 684 ✅ +4  81 💤 ±0  0 ❌ ±0 
5 774 runs  +4  5 693 ✅ +4  81 💤 ±0  0 ❌ ±0 

Results for commit f3830d7. ± Comparison against base commit aec7a69.

♻️ This comment has been updated with latest results.

@codecov-commenter

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@johnml1135 johnml1135 changed the title Fix LT-22351: OpenType/Graphite font features from style defaults now reach Dictionary Preview LT-22351: Preserve default font features in Preview and Word export Aug 13, 2026
Advance to the latest published LibLCM beta as the dependency baseline.

Remove the StyleInfo loader now that LibLCM owns persisted default features.

Cover Preview CSS and Word export precedence, and align the design record.
@johnml1135
johnml1135 marked this pull request as ready for review August 13, 2026 14:10
@jasonleenaylor

Copy link
Copy Markdown
Contributor

openspec/changes/archive/2026-07-14-add-opentype-font-features/design.md line 128 at r2 (raw file):

**Alternatives considered:** Remove the `StyleInfo` adapter immediately. Rejected for this change because `SaveToDB_DefaultFontFeatures_RoundTripsThroughRules` failed after removal. Broader LCM dependency alignment could retire the adapter later with the same round-trip tests as the gate.

**Status update (LT-22351):** The gating condition has been satisfied. liblcm's `BaseStyleInfo.ProcessStyleRules` now loads default `ktptFontVariations` (sillsdev/liblcm#388), `SaveToDB_DefaultFontFeatures_RoundTripsThroughRules` passes through the authoritative `BaseStyleInfo` path, and the `StyleInfo.LoadDefaultFontFeatures` adapter was removed under LT-22351.

I doubt that keeping the old decisions in a persistent document is actually good for future ai or human understanding. Modifying this to instead show the status quo seems a better option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants