From f3830d7f4c03b98bcfb89b088f9b45970404abfd Mon Sep 17 00:00:00 2001 From: John Lambert Date: Thu, 13 Aug 2026 05:19:32 -0400 Subject: [PATCH] Use LibLCM default font features throughout FieldWorks 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. --- Build/SilVersions.props | 4 +- Src/FwCoreDlgs/FwCoreDlgControls/StyleInfo.cs | 18 ---- Src/xWorks/xWorksTests/CssGeneratorTests.cs | 99 ++++++++++++++++++- .../xWorksTests/LcmWordGeneratorTests.cs | 77 +++++++++++++++ .../design.md | 10 +- 5 files changed, 183 insertions(+), 25 deletions(-) diff --git a/Build/SilVersions.props b/Build/SilVersions.props index b90cd986fc..6f9db561ad 100644 --- a/Build/SilVersions.props +++ b/Build/SilVersions.props @@ -12,7 +12,7 @@ ============================================================= --> - 11.0.0-beta0173 + 11.0.0-beta0178 18.0.0-beta0027 18.0.0-beta0012 6.0.0-beta0065 @@ -24,4 +24,4 @@ 70.1.152 60.0.56 - \ No newline at end of file + 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..e069e3f39b 100644 --- a/Src/xWorks/xWorksTests/CssGeneratorTests.cs +++ b/Src/xWorks/xWorksTests/CssGeneratorTests.cs @@ -2414,6 +2414,36 @@ public void GenerateCssForConfiguration_CustomPrintableAsciiFontFeatures_AreEsca Does.Contain("font-feature-settings:\"!abc\" 2,\"a\\\"b\\\\\" 1")); } + [Test] + public void GenerateCssForConfiguration_DefaultFontFeaturesFromPersistedStyleRules_ReachPreviewCss() + { + ConfiguredLcmGenerator.AssemblyFile = "xWorksTests"; + const string styleName = "DefaultFontFeaturesStyle"; + var persistedStyle = CreateStyleInfoFromPersistedRules(styleName, StyleType.kstCharacter, "smcp=1"); + SafelyAddStyleToSheetAndTable(styleName, persistedStyle); + try + { + 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")); + } + finally + { + // Fixture-owned collections outlive the model changes made by this test. + SafelyRemoveStyleFromSheetAndTable(styleName); + } + } + [Test] public void GenerateCssForConfiguration_ReversalSenseNumberWorks() { @@ -3150,6 +3180,50 @@ 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;")); } + [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" }; + + var persistedStyle = CreateStyleInfoFromPersistedRules(styleName, StyleType.kstParagraph, "smcp=1"); + SafelyAddStyleToSheetAndTable(styleName, persistedStyle); + 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", ""); + + Assert.That(cssResult, Contains.Substring("span[lang='" + vernWs.LanguageTag + "']{font-family:'Charis SIL',serif;font-feature-settings:\"smcp\" 1;")); + Assert.That(cssResult, Does.Not.Contain("ss11")); + } + finally + { + // Fixture-owned collections outlive the model changes made by this test. + SafelyRemoveStyleFromSheetAndTable(styleName); + } + } + [Test] public void GenerateCssForConfiguration_NormalStyleForWsDoesNotOverrideNodeStyle() { @@ -4126,7 +4200,22 @@ 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 BaseStyleInfo CreateStyleInfoFromPersistedRules(string name, StyleType type, string fontFeatures) + { + var style = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.StylesOC.Add(style); + style.Name = name; + style.Context = ContextValues.Internal; + style.Function = FunctionValues.Prose; + style.Structure = StructureValues.Undefined; + style.Type = type; + var propsBldr = TsStringUtils.MakePropsBldr(); + propsBldr.SetStrPropValue((int)FwTextPropType.ktptFontVariations, fontFeatures); + style.Rules = propsBldr.GetTextProps(); + return new BaseStyleInfo(style); + } + + private void SafelyAddStyleToSheetAndTable(string name, BaseStyleInfo style) { if (m_styleSheet.Styles.Contains(name)) m_styleSheet.Styles.Remove(name); @@ -4136,6 +4225,14 @@ private void SafelyAddStyleToSheetAndTable(string name, TestStyle style) m_owningTable.Add(name, style); } + 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(); diff --git a/Src/xWorks/xWorksTests/LcmWordGeneratorTests.cs b/Src/xWorks/xWorksTests/LcmWordGeneratorTests.cs index b535cc954c..31c8c2ee8c 100644 --- a/Src/xWorks/xWorksTests/LcmWordGeneratorTests.cs +++ b/Src/xWorks/xWorksTests/LcmWordGeneratorTests.cs @@ -328,6 +328,68 @@ public void GenerateCharacterStyleFromLcmStyleSheet_NormalStyle_UsesWritingSyste Is.EqualTo(new[] { W14.OnOffValues.True, W14.OnOffValues.True })); } + [Test] + public void GenerateCharacterStyleFromLcmStyleSheet_DefaultFontFeaturesFromPersistedStyleRules_AddsWordTypographyProperties() + { + var styleName = "WordFeatureStylePersisted" + Guid.NewGuid().ToString("N"); + var styles = FontHeightAdjuster.StyleSheetFromPropertyTable(m_propertyTable).Styles; + styles.Add(CreateStyleInfoFromPersistedRules(styleName, StyleType.kstCharacter, + "liga=0,lnum=1,pnum=1,calt=0,ss02=0,cv01=2")); + 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 + { + // Fixture-owned styles outlive the model changes made by this test. + styles.Remove(styleName); + } + } + + [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 styles = FontHeightAdjuster.StyleSheetFromPropertyTable(m_propertyTable).Styles; + if (styles.Contains(WordStylesGenerator.NormalParagraphStyleName)) + styles.Remove(WordStylesGenerator.NormalParagraphStyleName); + styles.Add(CreateStyleInfoFromPersistedRules(WordStylesGenerator.NormalParagraphStyleName, + StyleType.kstParagraph, "ss02=1")); + try + { + var style = WordStylesGenerator.GenerateCharacterStyleFromLcmStyleSheet( + WordStylesGenerator.NormalParagraphStyleName, + vernWs.Handle, + new ReadOnlyPropertyTable(m_propertyTable)); + + var runProps = style.GetFirstChild(); + Assert.That(runProps, Is.Not.Null); + + var runFonts = runProps.GetFirstChild(); + Assert.That(runFonts, Is.Not.Null); + Assert.That(runFonts.Ascii?.Value, Is.EqualTo("Charis SIL")); + + 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 + { + // Fixture-owned styles outlive the model changes made by this test. + styles.Remove(WordStylesGenerator.NormalParagraphStyleName); + styles.Add(new BaseStyleInfo { Name = WordStylesGenerator.NormalParagraphStyleName, IsParagraphStyle = true }); + } + } + [Test] [Category("ManualDocx")] public void GenerateManualDocxArtifact_CharisBaseline_NoFontOptions() @@ -350,6 +412,21 @@ public void GenerateManualDocxArtifact_CharisSs11Ss12() Assert.That(styleSetIds, Does.Contain(12U)); } + private BaseStyleInfo CreateStyleInfoFromPersistedRules(string name, StyleType type, string fontFeatures) + { + var style = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.StylesOC.Add(style); + style.Name = name; + style.Context = ContextValues.Internal; + style.Function = FunctionValues.Prose; + style.Structure = StructureValues.Undefined; + style.Type = type; + var propsBldr = TsStringUtils.MakePropsBldr(); + propsBldr.SetStrPropValue((int)FwTextPropType.ktptFontVariations, fontFeatures); + style.Rules = propsBldr.GetTextProps(); + return new BaseStyleInfo(style); + } + private static void AssertWordTypographyProperties(OpenXmlCompositeElement runProps, W14.LigaturesValues ligaturesValue, W14.NumberFormValues numberFormValue, W14.NumberSpacingValues numberSpacingValue, bool contextualAlternativesValue, diff --git a/openspec/changes/archive/2026-07-14-add-opentype-font-features/design.md b/openspec/changes/archive/2026-07-14-add-opentype-font-features/design.md index 14936d0767..5cd0fa93e4 100644 --- a/openspec/changes/archive/2026-07-14-add-opentype-font-features/design.md +++ b/openspec/changes/archive/2026-07-14-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. - 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