From a4f5234e8dc02ad06b42bfc2833008d8cd0ce2ad Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 10 Jul 2026 14:11:37 -0400 Subject: [PATCH 1/4] Fix LT-22351: rely on liblcm for default font features in Preview CSS liblcm's BaseStyleInfo.ProcessStyleRules previously dropped a style's default ktptFontVariations (font features) when loading from persisted Rules (sillsdev/liblcm#388). FieldWorks compensated for this in the Styles dialog only, via StyleInfo.LoadDefaultFontFeatures, which manually rescanned style.Rules for ktptFontVariations after calling the base constructor. The Dictionary Preview CSS path (CssGenerator.GenerateCssStyleFromLcmStyleSheet -> AddFontInfoCss) never went through StyleInfo -- it reads plain BaseStyleInfo objects straight out of LcmStyleSheet -- so it had no such adapter and silently lost default font features, meaning font-feature-settings never reached the generated preview CSS. Now that liblcm loads default font features itself, remove the FieldWorks-side adapter and let StyleInfo(IStStyle) rely entirely on BaseStyleInfo.ProcessStyleRules, as the LT-22324 design doc intended once the round-trip test passed through the authoritative liblcm path. Add a CssGenerator regression test that builds a style the way LcmStyleSheet does in production (a real IStStyle with persisted Rules, wrapped in BaseStyleInfo -- not a TestStyle double with an in-memory ExplicitValue) and asserts that default font features reach the generated CSS end-to-end. Co-Authored-By: Claude Fable 5 --- Src/FwCoreDlgs/FwCoreDlgControls/StyleInfo.cs | 18 ------- Src/xWorks/xWorksTests/CssGeneratorTests.cs | 52 +++++++++++++++++++ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/Src/FwCoreDlgs/FwCoreDlgControls/StyleInfo.cs b/Src/FwCoreDlgs/FwCoreDlgControls/StyleInfo.cs index d6a4f85833..0b7fb7dbeb 100644 --- a/Src/FwCoreDlgs/FwCoreDlgControls/StyleInfo.cs +++ b/Src/FwCoreDlgs/FwCoreDlgControls/StyleInfo.cs @@ -40,24 +40,6 @@ public class StyleInfo : BaseStyleInfo public StyleInfo(IStStyle style) : base(style) { - LoadDefaultFontFeatures(style); - } - - private void LoadDefaultFontFeatures(IStStyle style) - { - if (style == null || style.Rules == null) - return; - - for (int i = 0; i < style.Rules.StrPropCount; i++) - { - int tpt; - string value = style.Rules.GetStrProp(i, out tpt); - if (tpt == (int)FwTextPropType.ktptFontVariations) - { - m_defaultFontInfo.m_features.ExplicitValue = value; - return; - } - } } /// ------------------------------------------------------------------------------------ diff --git a/Src/xWorks/xWorksTests/CssGeneratorTests.cs b/Src/xWorks/xWorksTests/CssGeneratorTests.cs index 58870627bf..b5df3f700a 100644 --- a/Src/xWorks/xWorksTests/CssGeneratorTests.cs +++ b/Src/xWorks/xWorksTests/CssGeneratorTests.cs @@ -2414,6 +2414,58 @@ public void GenerateCssForConfiguration_CustomPrintableAsciiFontFeatures_AreEsca Does.Contain("font-feature-settings:\"!abc\" 2,\"a\\\"b\\\\\" 1")); } + /// + /// LT-22351: liblcm's BaseStyleInfo.ProcessStyleRules previously dropped a style's + /// default ktptFontVariations (font features) when loading a style from its persisted + /// Rules. FieldWorks had a Styles-dialog-only adapter (StyleInfo.LoadDefaultFontFeatures) + /// that compensated for this, but the Dictionary Preview CSS generation path + /// (GenerateCssStyleFromLcmStyleSheet/AddFontInfoCss) reads plain BaseStyleInfo objects + /// straight out of LcmStyleSheet and had no such adapter, so default font features never + /// reached the preview CSS. This test builds the style the same way LcmStyleSheet does in + /// production - a real IStStyle with persisted Rules wrapped directly in a BaseStyleInfo, + /// not a TestStyle double with ExplicitValue set in memory - to prove the fix in liblcm + /// (sillsdev/liblcm#388) makes it all the way to the generated CSS. + /// + [Test] + public void GenerateCssForConfiguration_DefaultFontFeaturesFromPersistedStyleRules_ReachPreviewCss() + { + ConfiguredLcmGenerator.AssemblyFile = "xWorksTests"; + const string styleName = "DefaultFontFeaturesStyle"; + var styleFactory = Cache.ServiceLocator.GetInstance(); + var realStyle = styleFactory.Create(); + Cache.LanguageProject.StylesOC.Add(realStyle); + realStyle.Name = styleName; + realStyle.Context = ContextValues.Internal; + realStyle.Function = FunctionValues.Prose; + realStyle.Structure = StructureValues.Undefined; + realStyle.Type = StyleType.kstCharacter; + + var propsBldr = TsStringUtils.MakePropsBldr(); + propsBldr.SetStrPropValue((int)FwTextPropType.ktptFontVariations, "smcp=1"); + realStyle.Rules = propsBldr.GetTextProps(); + + // Build the style the way LcmStyleSheet.LoadStyles does in production: a BaseStyleInfo + // wrapping the real, persisted IStStyle -- no TestStyle, no in-memory ExplicitValue. + var persistedStyle = new BaseStyleInfo(realStyle); + if (m_styleSheet.Styles.Contains(styleName)) + m_styleSheet.Styles.Remove(styleName); + m_styleSheet.Styles.Add(persistedStyle); + + var headwordNode = new ConfigurableDictionaryNode + { + FieldDescription = "SIL.FieldWorks.XWorks.TestRootClass", + Label = "Headword", + DictionaryNodeOptions = ConfiguredXHTMLGeneratorTests.GetWsOptionsForLanguages(new[] { "fr" }), + Style = styleName, + IsEnabled = true + }; + + var model = new DictionaryConfigurationModel { Parts = new List { headwordNode } }; + var cssResult = CssGenerator.GenerateCssFromConfiguration(model, m_propertyTable); + + Assert.That(cssResult, Does.Contain("font-feature-settings:\"smcp\" 1")); + } + [Test] public void GenerateCssForConfiguration_ReversalSenseNumberWorks() { From 9ea1da187501cfae4a367aeb82b9a5cfcc9485a9 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 10 Jul 2026 17:39:35 -0400 Subject: [PATCH 2/4] LT-22351 review: harden CssGeneratorTests; add precedence coverage Review follow-ups on the LT-22351 branch: - Widen SafelyAddStyleToSheetAndTable to accept BaseStyleInfo (both collections it touches already store BaseStyleInfo) and add a matching SafelyRemoveStyleFromSheetAndTable helper. - The persisted-Rules preview test now uses those helpers and removes its style in a finally block: m_styleSheet is per-fixture but the base class undoes all data changes per test, so leaving a BaseStyleInfo wrapping a real IStStyle behind would strand a stale entry with a dead RealStyle that can crash later tests order-dependently. - Add GenerateCssForConfiguration_NormalStyleOwnFontFeatures_ BeatWritingSystemDefaultFontFeatures: now that liblcm loads a style's own default ktptFontVariations from persisted Rules (sillsdev/liblcm#388), a Normal style WITH its own features must win over the writing system's DefaultFontFeatures fallback in AddFontInfoCss; the existing sibling test only covered Normal WITHOUT its own features. Co-Authored-By: Claude Fable 5 --- Src/xWorks/xWorksTests/CssGeneratorTests.cs | 112 +++++++++++++++++--- 1 file changed, 97 insertions(+), 15 deletions(-) diff --git a/Src/xWorks/xWorksTests/CssGeneratorTests.cs b/Src/xWorks/xWorksTests/CssGeneratorTests.cs index b5df3f700a..f660626a4c 100644 --- a/Src/xWorks/xWorksTests/CssGeneratorTests.cs +++ b/Src/xWorks/xWorksTests/CssGeneratorTests.cs @@ -2447,23 +2447,28 @@ public void GenerateCssForConfiguration_DefaultFontFeaturesFromPersistedStyleRul // Build the style the way LcmStyleSheet.LoadStyles does in production: a BaseStyleInfo // wrapping the real, persisted IStStyle -- no TestStyle, no in-memory ExplicitValue. var persistedStyle = new BaseStyleInfo(realStyle); - if (m_styleSheet.Styles.Contains(styleName)) - m_styleSheet.Styles.Remove(styleName); - m_styleSheet.Styles.Add(persistedStyle); - - var headwordNode = new ConfigurableDictionaryNode + SafelyAddStyleToSheetAndTable(styleName, persistedStyle); + try { - FieldDescription = "SIL.FieldWorks.XWorks.TestRootClass", - Label = "Headword", - DictionaryNodeOptions = ConfiguredXHTMLGeneratorTests.GetWsOptionsForLanguages(new[] { "fr" }), - Style = styleName, - IsEnabled = true - }; + var headwordNode = new ConfigurableDictionaryNode + { + FieldDescription = "SIL.FieldWorks.XWorks.TestRootClass", + Label = "Headword", + DictionaryNodeOptions = ConfiguredXHTMLGeneratorTests.GetWsOptionsForLanguages(new[] { "fr" }), + Style = styleName, + IsEnabled = true + }; - var model = new DictionaryConfigurationModel { Parts = new List { headwordNode } }; - var cssResult = CssGenerator.GenerateCssFromConfiguration(model, m_propertyTable); + var model = new DictionaryConfigurationModel { Parts = new List { headwordNode } }; + var cssResult = CssGenerator.GenerateCssFromConfiguration(model, m_propertyTable); - Assert.That(cssResult, Does.Contain("font-feature-settings:\"smcp\" 1")); + Assert.That(cssResult, Does.Contain("font-feature-settings:\"smcp\" 1")); + } + finally + { + // The real IStStyle is undone after this test; do not leave a stale entry behind. + SafelyRemoveStyleFromSheetAndTable(styleName); + } } [Test] @@ -3202,6 +3207,69 @@ public void GenerateCssForConfiguration_WsSpanWithNormalStyle_UsesWritingSystemD Assert.That(cssResult, Contains.Substring("span[lang='" + vernWs.LanguageTag + "']{font-family:'Charis SIL',serif;font-feature-settings:\"ss11\" 1,\"ss12\" 1;")); } + /// + /// LT-22351: now that liblcm loads a style's default ktptFontVariations from its persisted + /// Rules (sillsdev/liblcm#388), a Normal style with its OWN font features must win over the + /// writing system's DefaultFontFeatures fallback in AddFontInfoCss. The sibling test above + /// covers only a Normal style WITHOUT its own features (fallback branch). + /// + [Test] + public void GenerateCssForConfiguration_NormalStyleOwnFontFeatures_BeatWritingSystemDefaultFontFeatures() + { + const string styleName = "Normal"; + var vernWs = Cache.ServiceLocator.WritingSystemManager.Get(Cache.DefaultVernWs); + vernWs.DefaultFont = new FontDefinition("Charis SIL") { Features = "ss11=1,ss12=1" }; + + // A real IStStyle whose persisted Rules carry the style's own default font features. + var realStyle = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.StylesOC.Add(realStyle); + realStyle.Name = styleName; + realStyle.Context = ContextValues.Internal; + realStyle.Function = FunctionValues.Prose; + realStyle.Structure = StructureValues.Undefined; + realStyle.Type = StyleType.kstParagraph; + var propsBldr = TsStringUtils.MakePropsBldr(); + propsBldr.SetStrPropValue((int)FwTextPropType.ktptFontVariations, "smcp=1"); + realStyle.Rules = propsBldr.GetTextProps(); + + SafelyAddStyleToSheetAndTable(styleName, new BaseStyleInfo(realStyle)); + try + { + var glossNode = new ConfigurableDictionaryNode + { + FieldDescription = "Gloss", + DictionaryNodeOptions = ConfiguredXHTMLGeneratorTests.GetWsOptionsForLanguages(new[] { vernWs.LanguageTag }) + }; + var testSensesNode = new ConfigurableDictionaryNode + { + FieldDescription = "Senses", + Children = new List { glossNode } + }; + var testEntryNode = new ConfigurableDictionaryNode + { + FieldDescription = "LexEntry", + Children = new List { testSensesNode } + }; + var model = new DictionaryConfigurationModel + { + Parts = new List { testEntryNode } + }; + PopulateFieldsForTesting(testEntryNode); + + var cssResult = Regex.Replace(CssGenerator.GenerateCssFromConfiguration(model, m_propertyTable), @"\t|\n|\r", ""); + + // The style's own features win; the font family still falls back to the WS default font. + Assert.That(cssResult, Contains.Substring("span[lang='" + vernWs.LanguageTag + "']{font-family:'Charis SIL',serif;font-feature-settings:\"smcp\" 1;")); + // The WS DefaultFontFeatures fallback must not leak through. + Assert.That(cssResult, Does.Not.Contain("ss11")); + } + finally + { + // The real IStStyle is undone after this test; do not leave a stale entry behind. + SafelyRemoveStyleFromSheetAndTable(styleName); + } + } + [Test] public void GenerateCssForConfiguration_NormalStyleForWsDoesNotOverrideNodeStyle() { @@ -4178,7 +4246,7 @@ private static TestStyle GenerateStyleFromFontInfo(LcmCache cache, string name, return new TestStyle(fontInfo, cache) { Name = name, IsParagraphStyle = isParagraphStyle }; } - private void SafelyAddStyleToSheetAndTable(string name, TestStyle style) + private void SafelyAddStyleToSheetAndTable(string name, BaseStyleInfo style) { if (m_styleSheet.Styles.Contains(name)) m_styleSheet.Styles.Remove(name); @@ -4188,6 +4256,20 @@ private void SafelyAddStyleToSheetAndTable(string name, TestStyle style) m_owningTable.Add(name, style); } + /// + /// Removes a style added by SafelyAddStyleToSheetAndTable. Tests that add a BaseStyleInfo + /// wrapping a real IStStyle MUST call this in a finally block: m_styleSheet is created once + /// per fixture, but the base class undoes all data changes after each test, which would + /// leave a stale entry whose RealStyle is invalid and crash later tests order-dependently. + /// + private void SafelyRemoveStyleFromSheetAndTable(string name) + { + if (m_styleSheet.Styles.Contains(name)) + m_styleSheet.Styles.Remove(name); + if (m_owningTable.ContainsKey(name)) + m_owningTable.Remove(name); + } + private void GenerateBulletStyle(string name) { var fontInfo = new FontInfo(); From d3faf43d303faa1aabc18e18717148d526a88a00 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 10 Jul 2026 17:39:54 -0400 Subject: [PATCH 3/4] LT-22351 review: add Word export tests for font features and precedence The Word export path (WordStylesGenerator.AddFontInfoWordStyles) has the same precedence logic as the CSS path but had no coverage through the authoritative liblcm load path: the existing typography test used a TestStyle double with an in-memory ExplicitValue (bypassing BaseStyleInfo.ProcessStyleRules) and the existing Normal-style test only covered the writing-system DefaultFontFeatures fallback branch. Add two tests mirroring the CSS-side coverage: - GenerateCharacterStyleFromLcmStyleSheet_DefaultFontFeaturesFromPersisted StyleRules_AddsWordTypographyProperties: a real IStStyle whose persisted Rules carry ktptFontVariations, wrapped in a plain BaseStyleInfo the way LcmStyleSheet does in production, produces the expected w14 typography properties (sillsdev/liblcm#388). - GenerateCharacterStyleFromLcmStyleSheet_NormalStyleOwnFontFeatures_Beat WritingSystemDefaultFontFeatures: a Normal style with its own persisted features wins over the WS DefaultFontFeatures fallback (font family still falls back to the WS default font). Both tests clean up in finally blocks so no stale entry wrapping an undone IStStyle is left in the fixture-lifetime stylesheet. Co-Authored-By: Claude Fable 5 --- .../xWorksTests/LcmWordGeneratorTests.cs | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/Src/xWorks/xWorksTests/LcmWordGeneratorTests.cs b/Src/xWorks/xWorksTests/LcmWordGeneratorTests.cs index b535cc954c..598be37f2c 100644 --- a/Src/xWorks/xWorksTests/LcmWordGeneratorTests.cs +++ b/Src/xWorks/xWorksTests/LcmWordGeneratorTests.cs @@ -328,6 +328,103 @@ public void GenerateCharacterStyleFromLcmStyleSheet_NormalStyle_UsesWritingSyste Is.EqualTo(new[] { W14.OnOffValues.True, W14.OnOffValues.True })); } + /// + /// LT-22351: default font features must survive the load from a style's persisted Rules + /// (liblcm BaseStyleInfo.ProcessStyleRules, sillsdev/liblcm#388) and reach the generated + /// Word styles. The sibling test above uses a TestStyle double whose ExplicitValue is set + /// in memory, which bypasses ProcessStyleRules; this one builds the style the way + /// LcmStyleSheet does in production - a real IStStyle wrapped in a BaseStyleInfo. + /// + [Test] + public void GenerateCharacterStyleFromLcmStyleSheet_DefaultFontFeaturesFromPersistedStyleRules_AddsWordTypographyProperties() + { + var styleName = "WordFeatureStylePersisted" + Guid.NewGuid().ToString("N"); + var realStyle = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.StylesOC.Add(realStyle); + realStyle.Name = styleName; + realStyle.Context = ContextValues.Internal; + realStyle.Function = FunctionValues.Prose; + realStyle.Structure = StructureValues.Undefined; + realStyle.Type = StyleType.kstCharacter; + var propsBldr = TsStringUtils.MakePropsBldr(); + propsBldr.SetStrPropValue((int)FwTextPropType.ktptFontVariations, "liga=0,lnum=1,pnum=1,calt=0,ss02=0,cv01=2"); + realStyle.Rules = propsBldr.GetTextProps(); + + var styles = FontHeightAdjuster.StyleSheetFromPropertyTable(m_propertyTable).Styles; + styles.Add(new BaseStyleInfo(realStyle)); + try + { + var style = WordStylesGenerator.GenerateCharacterStyleFromLcmStyleSheet(styleName, Cache.DefaultVernWs, + new ReadOnlyPropertyTable(m_propertyTable)); + + var runProps = style.GetFirstChild(); + AssertWordTypographyProperties(runProps, W14.LigaturesValues.None, W14.NumberFormValues.Lining, + W14.NumberSpacingValues.Proportional, false, 2U, false); + } + finally + { + // The real IStStyle is undone after this test; do not leave a stale entry behind. + styles.Remove(styleName); + } + } + + /// + /// LT-22351: a Normal style with its OWN default font features (loaded from its persisted + /// Rules, sillsdev/liblcm#388) must win over the writing system's DefaultFontFeatures + /// fallback in AddFontInfoWordStyles. The sibling test above covers only a Normal style + /// WITHOUT its own features (fallback branch). + /// + [Test] + public void GenerateCharacterStyleFromLcmStyleSheet_NormalStyleOwnFontFeatures_BeatWritingSystemDefaultFontFeatures() + { + var vernWs = Cache.ServiceLocator.WritingSystemManager.Get(Cache.DefaultVernWs); + vernWs.DefaultFont = new FontDefinition("Charis SIL") { Features = "ss11=1,ss12=1" }; + + var realStyle = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.StylesOC.Add(realStyle); + realStyle.Name = WordStylesGenerator.NormalParagraphStyleName; + realStyle.Context = ContextValues.Internal; + realStyle.Function = FunctionValues.Prose; + realStyle.Structure = StructureValues.Undefined; + realStyle.Type = StyleType.kstParagraph; + var propsBldr = TsStringUtils.MakePropsBldr(); + propsBldr.SetStrPropValue((int)FwTextPropType.ktptFontVariations, "ss02=1"); + realStyle.Rules = propsBldr.GetTextProps(); + + var styles = FontHeightAdjuster.StyleSheetFromPropertyTable(m_propertyTable).Styles; + if (styles.Contains(WordStylesGenerator.NormalParagraphStyleName)) + styles.Remove(WordStylesGenerator.NormalParagraphStyleName); + styles.Add(new BaseStyleInfo(realStyle)); + try + { + var style = WordStylesGenerator.GenerateCharacterStyleFromLcmStyleSheet( + WordStylesGenerator.NormalParagraphStyleName, + vernWs.Handle, + new ReadOnlyPropertyTable(m_propertyTable)); + + var runProps = style.GetFirstChild(); + Assert.That(runProps, Is.Not.Null); + + // The font family still falls back to the WS default font... + var runFonts = runProps.GetFirstChild(); + Assert.That(runFonts, Is.Not.Null); + Assert.That(runFonts.Ascii?.Value, Is.EqualTo("Charis SIL")); + + // ...but the style's own features win over the WS DefaultFontFeatures fallback. + var stylisticSets = runProps.GetFirstChild(); + Assert.That(stylisticSets, Is.Not.Null); + var styleSet = stylisticSets.Elements().Single(); + Assert.That(styleSet.Id?.Value, Is.EqualTo(2U)); + Assert.That(styleSet.Val?.Value, Is.EqualTo(W14.OnOffValues.True)); + } + finally + { + // The real IStStyle is undone after this test; restore the fixture's Normal double. + styles.Remove(WordStylesGenerator.NormalParagraphStyleName); + styles.Add(new BaseStyleInfo { Name = WordStylesGenerator.NormalParagraphStyleName, IsParagraphStyle = true }); + } + } + [Test] [Category("ManualDocx")] public void GenerateManualDocxArtifact_CharisBaseline_NoFontOptions() From 5bc3727b360df0ecc770a9cba49ec7296752fd19 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 10 Jul 2026 17:40:10 -0400 Subject: [PATCH 4/4] LT-22351 review: record StyleInfo adapter removal in openspec docs design.md Decision 11, proposal.md, and research.md still described StyleInfo.LoadDefaultFontFeatures as an existing, load-bearing compatibility adapter. Update those passages to record that the adapter was removed under LT-22351 once liblcm's BaseStyleInfo.ProcessStyleRules gained the ktptFontVariations case (sillsdev/liblcm#388) and the Decision 11 gating condition (SaveToDB_DefaultFontFeatures_RoundTripsThroughRules passing through the authoritative path) was satisfied. Surgical wording updates only; no restructuring. Co-Authored-By: Claude Fable 5 --- openspec/changes/add-opentype-font-features/design.md | 10 ++++++---- .../changes/add-opentype-font-features/proposal.md | 2 +- .../changes/add-opentype-font-features/research.md | 9 ++++++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/openspec/changes/add-opentype-font-features/design.md b/openspec/changes/add-opentype-font-features/design.md index 14936d0767..fdaef880fb 100644 --- a/openspec/changes/add-opentype-font-features/design.md +++ b/openspec/changes/add-opentype-font-features/design.md @@ -17,7 +17,7 @@ The longer product phases are: add OpenType features now, remove Graphite later - Keep persisted feature strings renderer-neutral and compatible with future Avalonia/HarfBuzz-style consumption. - Accept any syntactically valid OpenType tag and reject malformed tags safely with trace logging. - Add trace logging for discovery, validation, native shaping, and fallback decisions. -- Keep style/default font-feature loading on the existing inheritance path, with only the minimal compatibility adapter still required by the current build graph. +- Keep style/default font-feature loading on the existing inheritance path (the interim `StyleInfo` compatibility adapter was later removed under LT-22351; see Decision 11). - Fix truncation and malformed-input robustness gaps in legacy feature-string handling. - Add tests for UI control behavior and visual rendering differences caused by feature toggles. - Add test-only HarfBuzzSharp + SkiaSharp comparison tooling for future visual-fidelity confidence. @@ -119,11 +119,13 @@ The longer product phases are: add OpenType features now, remove Graphite later ### 11. Existing inheritance paths remain authoritative -**Decision:** `FontInfo.m_features`, `FwTextPropType.ktptFontVariations`, and style rule round-tripping remain the authoritative inheritance/data-flow path for default and explicit font features. `StyleInfo` retains a minimal compatibility adapter that reads default `ktptFontVariations` from `IStStyle.Rules` because focused validation showed that removing it loses persisted default font features in the current build graph. +**Decision:** `FontInfo.m_features`, `FwTextPropType.ktptFontVariations`, and style rule round-tripping remain the authoritative inheritance/data-flow path for default and explicit font features. `StyleInfo` originally retained a minimal compatibility adapter that read default `ktptFontVariations` from `IStStyle.Rules` because focused validation showed that removing it lost persisted default font features in the then-current build graph. -**Rationale:** The local LCM source contains `BaseStyleInfo.ProcessStyleRules` support for `ktptFontVariations`, but the active FieldWorks build/test path still requires the `StyleInfo` adapter to reload persisted defaults. The adapter is therefore a compatibility boundary, not a second policy path. +**Rationale:** At the time of this decision, the active FieldWorks build/test path still required the `StyleInfo` adapter to reload persisted defaults even though the local LCM source contained `BaseStyleInfo.ProcessStyleRules` support for `ktptFontVariations`. The adapter was a compatibility boundary, not a second policy path. -**Alternatives considered:** Remove the `StyleInfo` adapter immediately. Rejected for this change because `SaveToDB_DefaultFontFeatures_RoundTripsThroughRules` failed after removal. Broader LCM dependency alignment can retire the adapter later with the same round-trip tests as the gate. +**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. ### 12. Overlong and malformed feature strings fail safe diff --git a/openspec/changes/add-opentype-font-features/proposal.md b/openspec/changes/add-opentype-font-features/proposal.md index 73eb77ec12..bf087d0ff7 100644 --- a/openspec/changes/add-opentype-font-features/proposal.md +++ b/openspec/changes/add-opentype-font-features/proposal.md @@ -16,7 +16,7 @@ The phase also needs explicit OpenType-first behavior for dual-technology fonts, - Add trace logging for malformed feature strings, malformed tags, filtered feature discovery, provider selection, native shaping failures, and fallback decisions. - Accept any syntactically valid OpenType tag name, including custom/private tags; reject malformed tags safely and log them instead of narrowing accepted tags to a registry allowlist. - Fix legacy truncation logic so overlong feature strings without comma boundaries fail safe. -- Keep the existing style/font-feature inheritance path authoritative and retain only the minimal `StyleInfo` compatibility adapter needed for the current build graph to reload default `ktptFontVariations` from style rules. +- Keep the existing style/font-feature inheritance path authoritative. (A minimal `StyleInfo` compatibility adapter originally reloaded default `ktptFontVariations` from style rules; it was removed under LT-22351 once liblcm's `BaseStyleInfo.ProcessStyleRules` gained the `ktptFontVariations` case — sillsdev/liblcm#388.) - Add UI/component tests for font-feature controls and high-level visual rendering tests proving feature settings change output. - Add robustness tests for malformed input, feature filtering, OpenType-preferred behavior, truncation safety, fallback behavior, CSS/DOCX export safety, and inheritance-path round-tripping. - Add a test-only HarfBuzzSharp + SkiaSharp comparison path for shaping/rendering confidence toward future Avalonia migration; this path is not a production renderer in Phase 1. diff --git a/openspec/changes/add-opentype-font-features/research.md b/openspec/changes/add-opentype-font-features/research.md index 7198d123c1..38ffef838d 100644 --- a/openspec/changes/add-opentype-font-features/research.md +++ b/openspec/changes/add-opentype-font-features/research.md @@ -265,9 +265,12 @@ Implementation follow-up: - Removing the `StyleInfo` loader was attempted during implementation and failed `SaveToDB_DefaultFontFeatures_RoundTripsThroughRules`, while restoring it made - the style/font-tab slice pass. The loader therefore remains as a minimal - compatibility adapter until the active LCM dependency path consumes - `ktptFontVariations` defaults through `BaseStyleInfo.ProcessStyleRules`. + the style/font-tab slice pass. The loader remained as a minimal compatibility + adapter until the active LCM dependency path consumed `ktptFontVariations` + defaults through `BaseStyleInfo.ProcessStyleRules`. That happened under + LT-22351: liblcm gained the `ktptFontVariations` case in `ProcessStyleRules` + (sillsdev/liblcm#388), the round-trip test passed through the authoritative + path, and the `StyleInfo.LoadDefaultFontFeatures` adapter was removed. ### Recommended Additional Tests Beyond The Original Plan