diff --git a/Src/Common/SimpleRootSite/EditingHelper.cs b/Src/Common/SimpleRootSite/EditingHelper.cs index 56f14d304f..b688ab2730 100644 --- a/Src/Common/SimpleRootSite/EditingHelper.cs +++ b/Src/Common/SimpleRootSite/EditingHelper.cs @@ -2842,9 +2842,11 @@ private void SetWritingSystemPropertyFromSelection(IVwRootBox rootbox, int oldWritingSystemHvo = int.Parse(s); if (oldWritingSystemHvo != ws) { + // Flag before the set: the handler must find it however the + // change is delivered. + m_fSuppressNextWritingSystemHvoChanged = true; rs.PropTable.SetProperty("WritingSystemHvo", ws.ToString(), true); rs.PropTable.SetPropertyPersistence("WritingSystemHvo", false); - m_fSuppressNextWritingSystemHvoChanged = true; } } } diff --git a/Src/LexText/Lexicon/LexEdDllTests/ReversalListenerTests.cs b/Src/LexText/Lexicon/LexEdDllTests/ReversalListenerTests.cs new file mode 100644 index 0000000000..217d52626b --- /dev/null +++ b/Src/LexText/Lexicon/LexEdDllTests/ReversalListenerTests.cs @@ -0,0 +1,39 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using NUnit.Framework; +using SIL.FieldWorks.XWorks.LexEd; +using SIL.LCModel; +using SIL.LCModel.Infrastructure; +using XCore; + +namespace LexEdDllTests +{ + [TestFixture] + public class ReversalListenerTests : MemoryOnlyBackendProviderTestBase + { + [Test] + public void Init_NoReversalIndexSelected_SelectsTheFirstIndexOnceItsTaskHasEnded() + { + var repository = Cache.ServiceLocator.GetInstance(); + IReversalIndex index = null; + NonUndoableUnitOfWorkHelper.Do(Cache.ActionHandlerAccessor, + () => index = repository.FindOrCreateIndexForWs(Cache.DefaultAnalWs)); + using (var mediator = new Mediator()) + using (var propertyTable = new PropertyTable(mediator)) + using (var listener = new ReversalListener()) + { + propertyTable.SetProperty("cache", Cache, false); + propertyTable.SetPropertyDispose("cache", false); + + listener.Init(mediator, propertyTable, null); + + Assert.That(propertyTable.GetStringProperty("ReversalIndexGuid", null), + Is.EqualTo(index.Guid.ToString())); + Assert.That(Cache.ActionHandlerAccessor.CurrentDepth, Is.EqualTo(0), + "Init ends the task it opened"); + } + } + } +} diff --git a/Src/LexText/Lexicon/ReversalListener.cs b/Src/LexText/Lexicon/ReversalListener.cs index f4733f29bd..ec5def8e48 100644 --- a/Src/LexText/Lexicon/ReversalListener.cs +++ b/Src/LexText/Lexicon/ReversalListener.cs @@ -194,21 +194,20 @@ public virtual void Init(Mediator mediator, PropertyTable propertyTable, XmlNode // Set up for the reversal index combo box or dropdown menu. var reversalIndexGuid = ReversalIndexEntryUi.GetObjectGuidIfValid(m_propertyTable, "ReversalIndexGuid"); + Guid firstGuid = Guid.Empty; if (reversalIndexGuid == Guid.Empty) { // We haven't established the reversal index yet. Choose the first one available. - Guid firstGuid = Guid.Empty; List reversalIds = cache.LanguageProject.LexDbOA.CurrentReversalIndices; if (reversalIds.Count > 0) firstGuid = reversalIds[0].Guid; else if (cache.LanguageProject.LexDbOA.ReversalIndexesOC.Count > 0) firstGuid = cache.LanguageProject.LexDbOA.ReversalIndexesOC.ToGuidArray()[0]; - if (firstGuid != Guid.Empty) - { - SetReversalIndexGuid(firstGuid); - } } cache.DomainDataByFlid.EndNonUndoableTask(); + // Set after the task so the change notification can never run inside it. + if (firstGuid != Guid.Empty) + SetReversalIndexGuid(firstGuid); } /// @@ -422,7 +421,9 @@ public override void ChangeOwningObject(Guid newGuid, bool updateAndNotify, Dict newGuid = Cache.ServiceLocator.GetInstance().AllInstances().First().Guid; if (updateAndNotify) { - m_propertyTable.SetProperty("ReversalIndexGuid", newGuid.ToString(), true); + // No broadcast: the only listener is a ReversalClerk, and this one + // is already changing to that index. + m_propertyTable.SetProperty("ReversalIndexGuid", newGuid.ToString(), false); } } diff --git a/Src/xWorks/DictionaryConfigurationMigrators/PreHistoricMigrator.cs b/Src/xWorks/DictionaryConfigurationMigrators/PreHistoricMigrator.cs index 0873cb2623..793343d12e 100644 --- a/Src/xWorks/DictionaryConfigurationMigrators/PreHistoricMigrator.cs +++ b/Src/xWorks/DictionaryConfigurationMigrators/PreHistoricMigrator.cs @@ -56,6 +56,14 @@ public class PreHistoricMigrator : IDictionaryConfigurationMigrator, ILayoutConv /// internal string m_configDirSuffixBeingMigrated; + /// + /// Layout selections (property name, converted configuration path) found while + /// migrating, written to the property table once the migration's unit of work + /// has ended. + /// + private readonly List> m_pendingLayoutSelections = + new List>(); + /// /// The mediator is needed to retrieve information about the prehistoric configurations /// @@ -97,17 +105,33 @@ public void MigrateIfNeeded(SimpleLogger logger, PropertyTable propertyTable, st m_logger.WriteLine("Migrating dictionary configurations"); m_configDirSuffixBeingMigrated = DictionaryConfigurationListener.DictConfigDirName; Directory.CreateDirectory(Path.Combine(projectPath, m_configDirSuffixBeingMigrated)); - UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW( - "Undo Migrate old Dictionary Configurations", "Redo Migrate old Dictionary Configurations", - Cache.ActionHandlerAccessor, PerformMigrationUOW); + MigrateInUnitOfWork("Undo Migrate old Dictionary Configurations", + "Redo Migrate old Dictionary Configurations"); m_logger.WriteLine(string.Format("Migrating Reversal Index configurations, if any - {0}", DateTime.Now.ToLongTimeString())); m_configDirSuffixBeingMigrated = DictionaryConfigurationListener.RevIndexConfigDirName; Directory.CreateDirectory(Path.Combine(projectPath, m_configDirSuffixBeingMigrated)); - UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW( - "Undo Migrate old Reversal Configurations", "Redo Migrate old Reversal Configurations", + MigrateInUnitOfWork("Undo Migrate old Reversal Configurations", + "Redo Migrate old Reversal Configurations"); + } + } + + /// + /// Runs PerformMigrationUOW in a unit of work, then writes the layout + /// selections it found. They are written even when the migration throws: + /// the converted files are already saved. + /// + private void MigrateInUnitOfWork(string undoText, string redoText) + { + try + { + UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW(undoText, redoText, Cache.ActionHandlerAccessor, PerformMigrationUOW); } + finally + { + ApplyPendingLayoutSelections(); + } } /// Perform the migration for Dictionary or Reversal (depending on m_configDirSuffixBeingMigrated. @@ -1028,16 +1052,54 @@ private bool LayoutOptionsMatchStyle(BaseStyleInfo style, XmlDocConfigureDlg.Lay return wsLabel.Split(',').Select(item => new DictionaryNodeListOptions.DictionaryNodeOption { Id = item.Trim(), IsEnabled = true }).ToList(); } - private void MigratePublicationLayoutSelection(string oldLayout, string newPath) + /// + /// Records that the configuration at replaces + /// as the selected layout, if that layout is + /// selected. + /// + /// Internal only for tests; production entry point is + /// MigrateIfNeeded() + internal void MigratePublicationLayoutSelection(string oldLayout, string newPath) { - if (oldLayout.Equals(m_propertyTable.GetStringProperty("DictionaryPublicationLayout", string.Empty))) + if (oldLayout.Equals(CurrentLayoutSelection("DictionaryPublicationLayout"))) + { + m_pendingLayoutSelections.Add( + new KeyValuePair("DictionaryPublicationLayout", newPath)); + } + else if (oldLayout.Equals(CurrentLayoutSelection("ReversalIndexPublicationLayout"))) { - m_propertyTable.SetProperty("DictionaryPublicationLayout", newPath, true); + m_pendingLayoutSelections.Add( + new KeyValuePair("ReversalIndexPublicationLayout", newPath)); } - else if (oldLayout.Equals(m_propertyTable.GetStringProperty("ReversalIndexPublicationLayout", string.Empty))) + } + + /// + /// The selected layout as the migration sees it: a pending selection shadows + /// the table. + /// + private string CurrentLayoutSelection(string propertyName) + { + var pending = m_pendingLayoutSelections.LastOrDefault( + selection => selection.Key == propertyName); + return pending.Key == propertyName + ? pending.Value + : m_propertyTable.GetStringProperty(propertyName, string.Empty); + } + + /// + /// Writes the recorded layout selections to the property table. Called outside + /// the migration's unit of work, so that a change notification can never run + /// inside it. + /// + /// Internal only for tests; production entry point is + /// MigrateIfNeeded() + internal void ApplyPendingLayoutSelections() + { + foreach (var selection in m_pendingLayoutSelections) { - m_propertyTable.SetProperty("ReversalIndexPublicationLayout", newPath, true); + m_propertyTable.SetProperty(selection.Key, selection.Value, true); } + m_pendingLayoutSelections.Clear(); } #region trivial portions of the ILayoutConverter implementation diff --git a/Src/xWorks/RecordClerk.cs b/Src/xWorks/RecordClerk.cs index 8c6370d299..5786008ce4 100644 --- a/Src/xWorks/RecordClerk.cs +++ b/Src/xWorks/RecordClerk.cs @@ -1994,16 +1994,18 @@ virtual public bool IsControllingTheRecordTreeBar oldActiveClerk?.BecomeInactive(); m_propertyTable.SetProperty("OldActiveClerk", oldActiveClerk, false); m_propertyTable.SetPropertyPersistence("OldActiveClerk", false); - m_propertyTable.SetProperty("ActiveClerk", this, true); - m_propertyTable.SetPropertyPersistence("ActiveClerk", false); // We are adding this property so that EntryDlgListener can get access to the owning object // without first getting a RecordClerk, since getting a RecordClerk at that level causes a // circular dependency in compilation. m_propertyTable.SetProperty("ActiveClerkOwningObject", OwningObject, false); m_propertyTable.SetPropertyPersistence("ActiveClerkOwningObject", false); + Cache.DomainDataByFlid.AddNotification(this); + // Both broadcasting sets come last, ActiveClerk first, so the + // activation is complete before a change notification runs. + m_propertyTable.SetProperty("ActiveClerk", this, true); + m_propertyTable.SetPropertyPersistence("ActiveClerk", false); m_propertyTable.SetProperty("ActiveClerkSelectedObject", CurrentObject, true); m_propertyTable.SetPropertyPersistence("ActiveClerkSelectedObject", false); - Cache.DomainDataByFlid.AddNotification(this); } } get diff --git a/Src/xWorks/XmlDocView.cs b/Src/xWorks/XmlDocView.cs index 9ef9829522..9dc589094a 100644 --- a/Src/xWorks/XmlDocView.cs +++ b/Src/xWorks/XmlDocView.cs @@ -565,6 +565,9 @@ protected override void SetupDataContext() //m_flid = RecordClerk.GetFlidOfVectorFromName(m_vectorName, Cache, out m_owningObject); RecordClerk clerk = Clerk; clerk.ActivateUI(false); + // This view never shows the record bar. Set once the clerk is active, + // before UpdateList, which reads it while populating the bar. + m_propertyTable.SetProperty("ShowRecordList", false, true); // Enhance JohnT: could use logic similar to RecordView.InitBase to load persisted list contents (filtered and sorted). if (clerk.RequestedLoadWhileSuppressed) clerk.UpdateList(false); @@ -757,8 +760,6 @@ protected void InitBase(Mediator mediator, PropertyTable propertyTable, XmlNode m_mediator.AddColleague(this); - m_propertyTable.SetProperty("ShowRecordList", false, true); - SetupDataContext(); ShowRecord(); } diff --git a/Src/xWorks/xWorksTests/DictionaryConfigurationMigrators/PreHistoricMigratorTests.cs b/Src/xWorks/xWorksTests/DictionaryConfigurationMigrators/PreHistoricMigratorTests.cs index 33e466a6f7..040c7715a8 100644 --- a/Src/xWorks/xWorksTests/DictionaryConfigurationMigrators/PreHistoricMigratorTests.cs +++ b/Src/xWorks/xWorksTests/DictionaryConfigurationMigrators/PreHistoricMigratorTests.cs @@ -2580,7 +2580,64 @@ public void TestMigrateCustomFieldNode() Assert.That(newTypeNode1.Label, Is.EqualTo("Single Sense"), "A custom field copies its label properly during conversion"); } + [Test] + public void MigratePublicationLayoutSelection_IsWrittenOnlyWhenApplied() + { + const string dictionary = "DictionaryPublicationLayout"; + const string newPath = "C:\\config\\Stem.fwdictconfig"; + try + { + m_propertyTable.SetProperty(dictionary, "publishStem", false); + + m_migrator.MigratePublicationLayoutSelection("publishStem", newPath); + Assert.That(SelectedLayout(dictionary), Is.EqualTo("publishStem"), + "The selection waits until the migration's unit of work has ended"); + + m_migrator.ApplyPendingLayoutSelections(); + Assert.That(SelectedLayout(dictionary), Is.EqualTo(newPath)); + } + finally + { + m_propertyTable.RemoveProperty(dictionary); + } + } + + [Test] + public void MigratePublicationLayoutSelection_PendingSelectionShadowsTheTable() + { + const string dictionary = "DictionaryPublicationLayout"; + const string reversal = "ReversalIndexPublicationLayout"; + const string stemPath = "C:\\config\\Stem.fwdictconfig"; + const string stemCopyPath = "C:\\config\\Stem-copy.fwdictconfig"; + const string reversalPath = "C:\\config\\en.fwdictconfig"; + try + { + m_propertyTable.SetProperty(dictionary, "publishStem", false); + m_propertyTable.SetProperty(reversal, "publishReversal-en", false); + + m_migrator.MigratePublicationLayoutSelection("publishStem", stemPath); + // A second layout type of the same name compares against the pending + // path, not the table. + m_migrator.MigratePublicationLayoutSelection("publishStem", stemCopyPath); + m_migrator.MigratePublicationLayoutSelection("publishReversal-en", reversalPath); + m_migrator.ApplyPendingLayoutSelections(); + + Assert.That(SelectedLayout(dictionary), Is.EqualTo(stemPath)); + Assert.That(SelectedLayout(reversal), Is.EqualTo(reversalPath)); + } + finally + { + m_propertyTable.RemoveProperty(dictionary); + m_propertyTable.RemoveProperty(reversal); + } + } + #region Helper + private string SelectedLayout(string property) + { + return m_propertyTable.GetStringProperty(property, null); + } + private void DeleteStyleSheet(string styleName) { var style = m_styleSheet.FindStyle(styleName);