Move property sets out of units of work and to sequence ends

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;
 				}
 			}
 		}

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<IReversalIndexRepository>();
+			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");
+			}
+		}
+	}
+}

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<IReversalIndex> 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);
 		}
 
 		/// <summary>
@@ -422,7 +421,9 @@ public override void ChangeOwningObject(Guid newGuid, bool updateAndNotify, Dict
 				newGuid = Cache.ServiceLocator.GetInstance<IReversalIndexRepository>().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);
 				}
 			}
 

Src/xWorks/DictionaryConfigurationMigrators/PreHistoricMigrator.cs

@@ -56,6 +56,14 @@ public class PreHistoricMigrator : IDictionaryConfigurationMigrator, ILayoutConv
 		/// </summary>
 		internal string m_configDirSuffixBeingMigrated;
 
+		/// <summary>
+		/// Layout selections (property name, converted configuration path) found while
+		/// migrating, written to the property table once the migration's unit of work
+		/// has ended.
+		/// </summary>
+		private readonly List<KeyValuePair<string, string>> m_pendingLayoutSelections =
+			new List<KeyValuePair<string, string>>();
+
 		/// <summary>
 		/// The mediator is needed to retrieve information about the prehistoric configurations
 		/// </summary>
@@ -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");
+			}
+		}
+
+		/// <summary>
+		/// 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.
+		/// </summary>
+		private void MigrateInUnitOfWork(string undoText, string redoText)
+		{
+			try
+			{
+				UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW(undoText, redoText,
 					Cache.ActionHandlerAccessor, PerformMigrationUOW);
 			}
+			finally
+			{
+				ApplyPendingLayoutSelections();
+			}
 		}
 
 		/// <summary>Perform the migration for Dictionary or Reversal (depending on m_configDirSuffixBeingMigrated.</summary>
@@ -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)
+		/// <summary>
+		/// Records that the configuration at <paramref name="newPath"/> replaces
+		/// <paramref name="oldLayout"/> as the selected layout, if that layout is
+		/// selected.
+		/// </summary>
+		/// <remarks>Internal only for tests; production entry point is
+		/// MigrateIfNeeded()</remarks>
+		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<string, string>("DictionaryPublicationLayout", newPath));
+			}
+			else if (oldLayout.Equals(CurrentLayoutSelection("ReversalIndexPublicationLayout")))
 			{
-				m_propertyTable.SetProperty("DictionaryPublicationLayout", newPath, true);
+				m_pendingLayoutSelections.Add(
+					new KeyValuePair<string, string>("ReversalIndexPublicationLayout", newPath));
 			}
-			else if (oldLayout.Equals(m_propertyTable.GetStringProperty("ReversalIndexPublicationLayout", string.Empty)))
+		}
+
+		/// <summary>
+		/// The selected layout as the migration sees it: a pending selection shadows
+		/// the table.
+		/// </summary>
+		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);
+		}
+
+		/// <summary>
+		/// 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.
+		/// </summary>
+		/// <remarks>Internal only for tests; production entry point is
+		/// MigrateIfNeeded()</remarks>
+		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

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

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();
 		}

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);