From f3ac5e37d8af04d47ee5e03eb5eb4a1a0c264c15 Mon Sep 17 00:00:00 2001 From: Jason Naylor Date: Tue, 29 Sep 2026 07:48:49 -0700 Subject: [PATCH 1/3] LT-22351: Offer Graphite features in the Styles dialog font tab Pass the selected writing system's Graphite setting to the Font Features button before the font name is applied, so a Graphite-enabled writing system lists Graphite features instead of the OpenType list. The default row covers every writing system, so it keeps the OpenType preference. Co-Authored-By: Claude Fable 5.1 --- .../FwCoreDlgControls/FwFontAttributes.cs | 12 ++++++++++ Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs | 22 ++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/Src/FwCoreDlgs/FwCoreDlgControls/FwFontAttributes.cs b/Src/FwCoreDlgs/FwCoreDlgControls/FwFontAttributes.cs index 7327c881b0..393b23bf0d 100644 --- a/Src/FwCoreDlgs/FwCoreDlgControls/FwFontAttributes.cs +++ b/Src/FwCoreDlgs/FwCoreDlgControls/FwFontAttributes.cs @@ -105,6 +105,18 @@ public string FontName set { m_btnFontFeatures.FontName = value; } } + /// ------------------------------------------------------------------------------------ + /// + /// Gets or sets whether the Font Features button offers Graphite features ahead of + /// OpenType features when the current font supports both. + /// + /// ------------------------------------------------------------------------------------ + public bool UseGraphiteFeatures + { + get { CheckDisposed(); return m_btnFontFeatures.UseGraphiteFeatures; } + set { CheckDisposed(); m_btnFontFeatures.UseGraphiteFeatures = value; } + } + /// ------------------------------------------------------------------------------------ /// /// Gets or sets a value indicating whether the controls for super/subscript are enabled or not. diff --git a/Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs b/Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs index c50498e8b3..eeaf8ebab4 100644 --- a/Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs +++ b/Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs @@ -45,6 +45,7 @@ public partial class FwFontTab : UserControl, IStylesTab private bool m_fIgnoreWsSelectedIndexChange = false; private StyleInfo m_currentStyleInfo; private int m_currentWs = -1; + private ILgWritingSystemFactory m_wsf; private bool m_fFontListIncludesRealNames = false; @@ -81,7 +82,12 @@ public void CheckDisposed() /// ------------------------------------------------------------------------------------ public ILgWritingSystemFactory WritingSystemFactory { - set { CheckDisposed(); m_FontAttributes.WritingSystemFactory = value; } + set + { + CheckDisposed(); + m_wsf = value; + m_FontAttributes.WritingSystemFactory = value; + } } #endregion @@ -302,6 +308,8 @@ public void UpdateForStyle(StyleInfo styleInfo, int ws) // only include the magic font names, not the real ones. FillFontNames(ws > -1); + m_FontAttributes.UseGraphiteFeatures = IsGraphiteEnabled(ws); + m_FontAttributes.ShowingInheritedProperties = true; // Always allow re-setting to unspecified for font attributes // Initialize controls based on whether or not this style inherits from another style. @@ -418,6 +426,18 @@ internal ComboBox FontNamesComboBox } #region private methods + /// + /// Returns whether Graphite rendering is enabled for the given writing system. The + /// default row (-1) covers every writing system, so it reports false. + /// + private bool IsGraphiteEnabled(int ws) + { + if (ws <= 0 || m_wsf == null) + return false; + var writingSystem = m_wsf.get_EngineOrNull(ws); + return writingSystem != null && writingSystem.IsGraphiteEnabled; + } + /// ------------------------------------------------------------------------------------ /// /// Fills the font names. From a14530085153002b13becc85c7b714adeaa4c136 Mon Sep 17 00:00:00 2001 From: Jason Naylor Date: Tue, 29 Sep 2026 07:48:49 -0700 Subject: [PATCH 2/3] LT-22351: Refresh views when a writing system's font settings change Raise the writing system model's update event when the default font, its font features, the Graphite setting, the text direction, or the numbering system changes, not only when the abbreviation or id changes. The Lexicon Edit dictionary preview keeps the page it generated earlier until a refresh, so a feature chosen in Writing System Properties never reached it. Raise the event once, after the save's unit of work closes, and skip it when the list update already refreshes. Also stop the font tab writing the Graphite flag onto a writing system just for displaying it, which marked an untouched writing system changed. Co-Authored-By: Claude Fable 5.1 --- .../FwCoreDlgControls/DefaultFontsControl.cs | 5 +- .../FwWritingSystemSetupModelTests.cs | 133 ++++++++++++++++++ Src/FwCoreDlgs/FwWritingSystemSetupModel.cs | 39 ++++- 3 files changed, 168 insertions(+), 9 deletions(-) diff --git a/Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs b/Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs index 08944e51c1..fecc2164c2 100644 --- a/Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs +++ b/Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs @@ -250,9 +250,8 @@ protected void SetSelectedFonts() bool isGraphiteFont = m_defaultFontFeaturesButton.IsGraphiteFont; m_graphiteGroupBox.Enabled = isGraphiteFont || m_defaultFontFeaturesButton.HasFontFeatures; m_enableGraphiteCheckBox.Enabled = isGraphiteFont; - if (!isGraphiteFont) - m_ws.IsGraphiteEnabled = false; - m_enableGraphiteCheckBox.Checked = m_ws.IsGraphiteEnabled; + // Display only; writing the flag here would mark an untouched writing system changed. + m_enableGraphiteCheckBox.Checked = isGraphiteFont && m_ws.IsGraphiteEnabled; } /// diff --git a/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs b/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs index 7ca12f3f21..189e4ac803 100644 --- a/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs +++ b/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs @@ -760,6 +760,139 @@ public void Model_WritingSystemChanged_NotCalledOnIrrelevantChange() Assert.That(writingSystemChanged, Is.False, "WritingSystemUpdated should not have been called after this change"); } + [Test] + public void Model_WritingSystemChanged_CalledOnDefaultFontFeaturesChange() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL"); + testModel.Save(); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + // Changing only the font features must refresh views that cache rendered output. + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL") { Features = "cv43=1,smcp=1" }; + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_CalledOnGraphiteToggle() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL"); + testModel.IsGraphiteEnabled = false; + testModel.Save(); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + testModel.IsGraphiteEnabled = true; + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_CalledOnRightToLeftChange() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + testModel.CurrentWsSetupModel.CurrentRightToLeftScript = true; + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_CalledOnNumberingSystemChange() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + testModel.CurrentWsSetupModel.CurrentNumberingSystemDefinition = NumberingSystemDefinition.CreateCustomSystem("abcdefghij"); + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_NotCalledOnReorderedFontFeatures() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL") { Features = "smcp=1,cv43=1" }; + testModel.Save(); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL") { Features = "cv43=1,smcp=1" }; + testModel.Save(); + Assert.That(writingSystemChanged, Is.False, "Reordering the same features is not a rendering change"); + } + + [Test] + public void Model_WritingSystemChanged_NotCalledWhenListAlsoChanged() + { + var writingSystemChanged = false; + var listChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr", "en" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.WritingSystemUpdated += (sender, args) => { writingSystemChanged = true; }; + testModel.WritingSystemListUpdated += (sender, args) => { listChanged = true; }; + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL"); + testModel.MoveDown(); + testModel.Save(); + Assert.That(listChanged, Is.True, "WritingSystemListUpdated should have been called after this change"); + Assert.That(writingSystemChanged, Is.False, "One refresh is enough when the list update already refreshes"); + } + + [Test] + public void Model_WritingSystemChanged_RaisedAfterUnitOfWorkCloses() + { + Cache.ActionHandlerAccessor.EndUndoTask(); + var testModel = new FwWritingSystemSetupModel(Cache.LangProject, FwWritingSystemSetupModel.ListType.Vernacular, + Cache.ServiceLocator.WritingSystemManager, Cache); + var depthWhenRaised = -1; + testModel.WritingSystemUpdated += (sender, args) => + { + depthWhenRaised = Cache.ActionHandlerAccessor.CurrentDepth; + }; + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL") { Features = "cv43=1,smcp=1" }; + testModel.Save(); + Assert.That(depthWhenRaised, Is.EqualTo(0), "Listeners refresh views, which must not run inside the save's unit of work"); + } + [TestCase(FwWritingSystemSetupModel.ListType.Vernacular)] [TestCase(FwWritingSystemSetupModel.ListType.Analysis)] public void WritingSystemTitle_ChangesByType(FwWritingSystemSetupModel.ListType type) diff --git a/Src/FwCoreDlgs/FwWritingSystemSetupModel.cs b/Src/FwCoreDlgs/FwWritingSystemSetupModel.cs index 52ad084022..a710d61ab9 100644 --- a/Src/FwCoreDlgs/FwWritingSystemSetupModel.cs +++ b/Src/FwCoreDlgs/FwWritingSystemSetupModel.cs @@ -80,7 +80,8 @@ public List WorkingList public event EventHandler WritingSystemListUpdated; /// - /// This event is fired only when the id or abbreviation for a writing system changes + /// Raised once after a save when an existing writing system's id, abbreviation, or + /// rendering settings changed. Not raised when WritingSystemListUpdated is. /// public event EventHandler WritingSystemUpdated; @@ -663,6 +664,9 @@ public void Save() { uowHelper = new NonUndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor); } + // Listeners refresh every window, so notify once, after the unit of work has closed. + var needsRefresh = false; + var listChanged = false; try { @@ -715,6 +719,7 @@ public void Save() else if (workingWs.IsChanged) { var didAbbrevOrIdChange = origWs.Abbreviation != workingWs.Abbreviation; + var didRenderingChange = RenderingChanged(origWs, workingWs); var oldId = origWs.Id; var oldHandle = origWs.Handle; // copy the working writing system content into the original writing system @@ -729,9 +734,9 @@ public void Save() } didAbbrevOrIdChange = true; } - if (didAbbrevOrIdChange) + if (didAbbrevOrIdChange || didRenderingChange) { - WritingSystemUpdated?.Invoke(this, EventArgs.Empty); + needsRefresh = true; } } @@ -767,13 +772,35 @@ public void Save() } finally { - if (CurrentWsListChanged) + listChanged = CurrentWsListChanged; + if (uowHelper != null) + uowHelper.Dispose(); + if (listChanged) { WritingSystemListUpdated?.Invoke(this, EventArgs.Empty); } - if (uowHelper != null) - uowHelper.Dispose(); } + if (needsRefresh && !listChanged) + { + WritingSystemUpdated?.Invoke(this, EventArgs.Empty); + } + } + + /// + /// Returns whether the two definitions would render text differently: another default + /// font, other font features once normalized, or a different Graphite setting, text + /// direction, or numbering system. + /// + private static bool RenderingChanged(CoreWritingSystemDefinition origWs, CoreWritingSystemDefinition workingWs) + { + if (origWs.DefaultFontName != workingWs.DefaultFontName) + return true; + if (FontFeatureSettings.NormalizePreservingLegacy(origWs.DefaultFontFeatures) + != FontFeatureSettings.NormalizePreservingLegacy(workingWs.DefaultFontFeatures)) + return true; + return origWs.IsGraphiteEnabled != workingWs.IsGraphiteEnabled + || origWs.RightToLeftScript != workingWs.RightToLeftScript + || !Equals(origWs.NumberingSystem, workingWs.NumberingSystem); } private static void AddOrMoveInList(ICollection allWritingSystems, int desiredIndex, CoreWritingSystemDefinition workingWs) From a0070f8549530bbf382747f4d947039645b921a3 Mon Sep 17 00:00:00 2001 From: Jason Naylor Date: Tue, 29 Sep 2026 14:44:20 -0700 Subject: [PATCH 3/3] LT-22351: Test the refresh when font features change in place The writing system dialog writes new features onto the existing FontDefinition, so the model test now does the same rather than swapping in a new definition, and checks that Save still raises WritingSystemUpdated. Co-Authored-By: Claude Fable 5.1 --- .../FwWritingSystemSetupModelTests.cs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs b/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs index 189e4ac803..85cdd0a937 100644 --- a/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs +++ b/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs @@ -781,6 +781,27 @@ public void Model_WritingSystemChanged_CalledOnDefaultFontFeaturesChange() Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); } + [Test] + public void Model_WritingSystemChanged_CalledOnInPlaceFontFeaturesChange() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL"); + testModel.Save(); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + // The font dialog edits the existing definition rather than replacing it. + testModel.CurrentDefaultFont.Features = "cv43=1"; + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + [Test] public void Model_WritingSystemChanged_CalledOnGraphiteToggle() {