diff --git a/Src/FdoUi/ReversalIndexEntryUi.cs b/Src/FdoUi/ReversalIndexEntryUi.cs index 5b4f03250c..dda8c2e380 100644 --- a/Src/FdoUi/ReversalIndexEntryUi.cs +++ b/Src/FdoUi/ReversalIndexEntryUi.cs @@ -46,6 +46,20 @@ protected override DummyCmObject GetMergeinfo(WindowParams wp, List + /// PropertyTable key naming the reversal index whose entries the clerk loads. + /// + public const string ReversalIndexGuidProperty = "ReversalIndexGuid"; + + /// + /// Fetches the GUID of the reversal index named by the ReversalIndexGuid property. + /// + /// The reversal index GUID, or empty GUID if there is a problem + public static Guid GetReversalIndexGuid(PropertyTable propertyTable) + { + return GetObjectGuidIfValid(propertyTable, ReversalIndexGuidProperty); + } + /// /// Fetches the GUID value of the given property, having checked it is a valid object. /// If it is not a valid object, the property is removed. diff --git a/Src/xWorks/XhtmlDocView.cs b/Src/xWorks/XhtmlDocView.cs index a74e556a3d..a460ce9374 100644 --- a/Src/xWorks/XhtmlDocView.cs +++ b/Src/xWorks/XhtmlDocView.cs @@ -9,10 +9,12 @@ using SIL.FieldWorks.Common.FwUtils; using SIL.FieldWorks.Common.RootSites; using SIL.FieldWorks.Common.Widgets; +using SIL.FieldWorks.FdoUi; using SIL.FieldWorks.FwCoreDlgControls; using SIL.FieldWorks.FwCoreDlgs; using SIL.IO; using SIL.LCModel; +using SIL.LCModel.Core.WritingSystems; using SIL.LCModel.DomainServices; using SIL.LCModel.Utils; using SIL.Progress; @@ -45,6 +47,10 @@ internal class XhtmlDocView : XWorksViewBase, IFindAndReplaceContext, IPostLayou internal string m_configObjectName; internal const string CurrentSelectedEntryClass = "currentSelectedEntry"; private const string ClassifiedDictConfig = "SemanticDomainSenses.fwdictconfig"; + /// + /// Id of the reversal clerk, declared in Configuration/Lexicon/areaConfiguration.xml. + /// + private const string AllReversalEntriesClerkId = "AllReversalEntries"; private const string FieldWorksPrintLimitEnv = "FIELDWORKS_PRINT_LIMIT"; private bool m_updateContentLater = false; // Whether we should postpone calling UpdateContent private string m_loadedConfig = null; @@ -73,7 +79,7 @@ public override void Init(Mediator mediator, PropertyTable propertyTable, XmlNod if (m_mainView.NativeBrowser is GeckoWebBrowser browser) { var clerk = XmlUtils.GetOptionalAttributeValue(configurationParameters, "clerk"); - if (clerk == "entries" || clerk == "AllReversalEntries" || clerk == "SemanticDomainList") + if (clerk == "entries" || clerk == AllReversalEntriesClerkId || clerk == "SemanticDomainList") { browser.DomClick += OnDomClick; browser.DomKeyPress += OnDomKeyPress; @@ -1125,9 +1131,10 @@ public void OnPropertyChanged(string name) var currentConfig = GetCurrentConfiguration(false); if (name == "ReversalIndexPublicationLayout") { - DictionaryConfigurationUtils.SetReversalIndexGuidBasedOnReversalIndexConfiguration(m_propertyTable, Cache); - // Wait until SetActiveSelectedEntryOnView to call UpdateContent. - m_updateContentLater = true; + // Sync ReversalIndexGuid to the chosen configuration. True means a + // different index, so let the clerk's reload refresh the view; false + // means no reload is coming, so refresh below. + m_updateContentLater = SyncReversalIndexGuidToConfiguration(); } var currentPublication = GetCurrentPublication(); var validPublication = GetValidPublicationForConfiguration(currentConfig) ?? xWorksStrings.AllEntriesPublication; @@ -1191,6 +1198,67 @@ private void RemoveStyleFromPreviousSelectedEntryOnView(GeckoWebBrowser browser) } } + /// + /// Points the ReversalIndexGuid property at the reversal index described by the current + /// ReversalIndexPublicationLayout. The index comes from the configuration's writing + /// system, so two configurations of one writing system select the same index. + /// + /// true if the selected index changed. Setting the property has + /// already told the clerk to reload, so ActiveClerkSelectedObject will follow on + /// Idle; the caller can defer refreshing until then. + internal bool SyncReversalIndexGuidToConfiguration() + { + var oldGuid = ReversalIndexEntryUi.GetReversalIndexGuid(m_propertyTable); + DictionaryConfigurationUtils.SetReversalIndexGuidBasedOnReversalIndexConfiguration(m_propertyTable, Cache); + return ReversalIndexEntryUi.GetReversalIndexGuid(m_propertyTable) != oldGuid; + } + + /// + /// Gets the writing system id of the reversal index named by the ReversalIndexGuid + /// property, or null if no valid index is selected. + /// + private string GetSelectedReversalWritingSystemId() + { + var reversalIndexGuid = ReversalIndexEntryUi.GetReversalIndexGuid(m_propertyTable); + if (reversalIndexGuid.Equals(Guid.Empty)) + return null; + var reversalIndex = Cache.ServiceLocator.GetObject(reversalIndexGuid) as IReversalIndex; + CoreWritingSystemDefinition writingSystem; + if (reversalIndex == null || !Cache.ServiceLocator.WritingSystemManager.TryGet(reversalIndex.WritingSystem, out writingSystem)) + return null; + return writingSystem.Id; + } + + /// + /// Decides whether ReversalIndexPublicationLayout still describes the reversal index + /// that is selected, and if not, works out what it should be replaced with. + /// + /// Writing system of the clerk's current + /// entry, used only when no valid index is selected. + /// Folder holding this project's reversal + /// configurations. + /// The configuration to switch to, or + /// null when the expected writing system has no configuration file. + /// true if the property needs to be rewritten. + internal bool TryGetReplacementReversalConfiguration(string currentEntryWritingSystemId, + string projectConfigurationDirectory, out string replacementConfiguration) + { + // Take the writing system from ReversalIndexGuid, the selection the user made; the + // clerk's current entry can still belong to the index we are leaving (LT-22642). + var expectedWritingSystemId = GetSelectedReversalWritingSystemId() ?? currentEntryWritingSystemId; + var currentConfiguration = m_propertyTable.GetStringProperty("ReversalIndexPublicationLayout", string.Empty); + if (File.Exists(currentConfiguration) + && new DictionaryConfigurationModel(currentConfiguration, Cache).WritingSystem == expectedWritingSystemId) + { + replacementConfiguration = null; + return false; + } + var newConfiguration = Path.Combine(projectConfigurationDirectory, + expectedWritingSystemId + DictionaryConfigurationModel.FileExtension); + replacementConfiguration = File.Exists(newConfiguration) ? newConfiguration : null; + return true; + } + /// /// Set the style attribute on the current entry to color the background. /// @@ -1198,7 +1266,7 @@ private void SetActiveSelectedEntryOnView(GeckoWebBrowser browser) { if (Clerk.CurrentObject == null) { - if (Clerk.Id == "AllReversalEntries" && m_updateContentLater) + if (Clerk.Id == AllReversalEntriesClerkId && m_updateContentLater) { // There are no entries, but we still need to clear the pane and update the title. var currentConfig = m_propertyTable.GetStringProperty("ReversalIndexPublicationLayout", string.Empty); @@ -1209,7 +1277,7 @@ private void SetActiveSelectedEntryOnView(GeckoWebBrowser browser) return; } - if (Clerk.Id == "AllReversalEntries") + if (Clerk.Id == AllReversalEntriesClerkId) { var reversalentry = Clerk.CurrentObject as IReversalIndexEntry; if (reversalentry == null) @@ -1219,13 +1287,12 @@ private void SetActiveSelectedEntryOnView(GeckoWebBrowser browser) return; var currReversalWs = writingSystem.Id; var currentConfig = m_propertyTable.GetStringProperty("ReversalIndexPublicationLayout", string.Empty); - var configuration = File.Exists(currentConfig) ? new DictionaryConfigurationModel(currentConfig, Cache) : null; var currentPage = GetTopCurrentPageButton(browser.Document.Body); - if (configuration == null || configuration.WritingSystem != currReversalWs) + string newConfig; + if (TryGetReplacementReversalConfiguration(currReversalWs, + DictionaryConfigurationListener.GetProjectConfigurationDirectory(m_propertyTable, reversalentry), out newConfig)) { - var newConfig = Path.Combine(DictionaryConfigurationListener.GetProjectConfigurationDirectory(m_propertyTable, reversalentry), - writingSystem.Id + DictionaryConfigurationModel.FileExtension); - m_propertyTable.SetProperty("ReversalIndexPublicationLayout", File.Exists(newConfig) ? newConfig : null, true); + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", newConfig, true); } else if (m_updateContentLater) { // Force the content to be updated once (LT-21702). @@ -1350,6 +1417,11 @@ private void RefreshCurrentView(object obj) private void RefreshAllContent(object _) { var currentConfig = GetCurrentConfiguration(false); + if (Clerk.Id == AllReversalEntriesClerkId) + { + // The Configure dialog sets the layout silently, so sync the guid. + SyncReversalIndexGuidToConfiguration(); + } var currentPublication = GetCurrentPublication(); var validPublication = GetValidPublicationForConfiguration(currentConfig) ?? xWorksStrings.AllEntriesPublication; if (currentPublication != xWorksStrings.AllEntriesPublication && currentPublication != validPublication) diff --git a/Src/xWorks/xWorksTests/XhtmlDocViewTests.cs b/Src/xWorks/xWorksTests/XhtmlDocViewTests.cs index d63191bf35..2019234ac0 100644 --- a/Src/xWorks/xWorksTests/XhtmlDocViewTests.cs +++ b/Src/xWorks/xWorksTests/XhtmlDocViewTests.cs @@ -8,7 +8,9 @@ using System.Xml; using NUnit.Framework; using SIL.FieldWorks.Common.RootSites; +using SIL.FieldWorks.FdoUi; using SIL.LCModel.Core.Text; +using SIL.LCModel.Core.WritingSystems; using SIL.IO; using SIL.FieldWorks.Common.FwUtils; using SIL.LCModel; @@ -431,6 +433,228 @@ public void GetValidConfigurationForPublication_ConfigurationContainingPubIsPick } } + #region Reversal configuration switching (LT-22642) + + private const string ReversalConfigDirName = "XhtmlDocViewReversalConfigs"; + + /// + /// Writes a minimal reversal configuration file for . + /// + private static void WriteReversalConfiguration(string path, string label, string writingSystemId) + { + new DictionaryConfigurationModel + { + FilePath = path, + Label = label, + WritingSystem = writingSystemId, + Parts = new List(), + SharedItems = new List(), + Publications = new List() + }.Save(); + } + + /// + /// Makes an analysis writing system and returns + /// its reversal index, creating the index if the project does not have one yet. + /// + private IReversalIndex CreateReversalIndex(string writingSystemId) + { + CoreWritingSystemDefinition writingSystem; + Cache.ServiceLocator.WritingSystemManager.GetOrSet(writingSystemId, out writingSystem); + if (!Cache.ServiceLocator.WritingSystems.AnalysisWritingSystems.Contains(writingSystem)) + Cache.ServiceLocator.WritingSystems.AnalysisWritingSystems.Add(writingSystem); + return Cache.ServiceLocator.GetInstance().FindOrCreateIndexForWs(writingSystem.Handle); + } + + private static string CreateReversalConfigDirectory() + { + var directory = Path.Combine(Path.GetTempPath(), ReversalConfigDirName); + if (Directory.Exists(directory)) + Directory.Delete(directory, true); + Directory.CreateDirectory(directory); + return directory; + } + + [Test] + public void SyncReversalIndexGuidToConfiguration_SecondConfigurationOfSameWritingSystem_ReportsNoIndexChange() + { + var configDirectory = CreateReversalConfigDirectory(); + try + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + using (var docView = new TestXhtmlDocView()) + { + CreateReversalIndex("en"); + var printConfig = Path.Combine(configDirectory, "en-print" + DictionaryConfigurationModel.FileExtension); + var webonaryConfig = Path.Combine(configDirectory, "en-webonary" + DictionaryConfigurationModel.FileExtension); + WriteReversalConfiguration(printConfig, "English-print", "en"); + WriteReversalConfiguration(webonaryConfig, "English-Webonary", "en"); + docView.SetPropertyTable(m_propertyTable); + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", printConfig, false); + Assert.That(docView.SyncReversalIndexGuidToConfiguration(), Is.True, + "Choosing the first configuration should select its reversal index"); + var guidAfterFirstChoice = m_propertyTable.GetStringProperty(ReversalIndexEntryUi.ReversalIndexGuidProperty, string.Empty); + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", webonaryConfig, false); + + // SUT + var indexChanged = docView.SyncReversalIndexGuidToConfiguration(); + + Assert.That(indexChanged, Is.False, + "Both configurations use 'en', so the index does not change, no clerk reload follows, and the view must refresh right away"); + Assert.That(m_propertyTable.GetStringProperty(ReversalIndexEntryUi.ReversalIndexGuidProperty, string.Empty), Is.EqualTo(guidAfterFirstChoice)); + } + } + finally + { + Directory.Delete(configDirectory, true); + } + } + + [Test] + public void SyncReversalIndexGuidToConfiguration_ConfigurationOfOtherWritingSystem_ReportsIndexChange() + { + var configDirectory = CreateReversalConfigDirectory(); + try + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + using (var docView = new TestXhtmlDocView()) + { + CreateReversalIndex("en"); + var spanishIndex = CreateReversalIndex("es"); + var englishConfig = Path.Combine(configDirectory, "en" + DictionaryConfigurationModel.FileExtension); + var spanishConfig = Path.Combine(configDirectory, "es" + DictionaryConfigurationModel.FileExtension); + WriteReversalConfiguration(englishConfig, "English-print", "en"); + WriteReversalConfiguration(spanishConfig, "Spanish", "es"); + docView.SetPropertyTable(m_propertyTable); + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", englishConfig, false); + docView.SyncReversalIndexGuidToConfiguration(); + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", spanishConfig, false); + + // SUT + var indexChanged = docView.SyncReversalIndexGuidToConfiguration(); + + Assert.That(indexChanged, Is.True, "Switching writing systems selects a different reversal index"); + Assert.That(m_propertyTable.GetStringProperty(ReversalIndexEntryUi.ReversalIndexGuidProperty, string.Empty), + Is.EqualTo(spanishIndex.Guid.ToString())); + } + } + finally + { + Directory.Delete(configDirectory, true); + } + } + + [Test] + public void TryGetReplacementReversalConfiguration_ConfigurationMatchesSelectedIndex_KeepsUserChoice() + { + var configDirectory = CreateReversalConfigDirectory(); + try + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + using (var docView = new TestXhtmlDocView()) + { + CreateReversalIndex("en"); + var spanishIndex = CreateReversalIndex("es"); + var englishConfig = Path.Combine(configDirectory, "en" + DictionaryConfigurationModel.FileExtension); + var spanishConfig = Path.Combine(configDirectory, "es" + DictionaryConfigurationModel.FileExtension); + WriteReversalConfiguration(englishConfig, "English-print", "en"); + WriteReversalConfiguration(spanishConfig, "Spanish", "es"); + docView.SetPropertyTable(m_propertyTable); + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", spanishConfig, false); + m_propertyTable.SetProperty(ReversalIndexEntryUi.ReversalIndexGuidProperty, spanishIndex.Guid.ToString(), false); + + // SUT: the clerk still holds an English entry while the switch to Spanish + // is in flight + string replacement; + var needsRewrite = docView.TryGetReplacementReversalConfiguration("en", configDirectory, out replacement); + + Assert.That(needsRewrite, Is.False, + "The configuration matches the selected index, so the choice the user made must not be overwritten"); + Assert.That(replacement, Is.Null); + } + } + finally + { + Directory.Delete(configDirectory, true); + } + } + + [Test] + public void TryGetReplacementReversalConfiguration_ConfigurationOfOtherIndex_IsReplaced() + { + var configDirectory = CreateReversalConfigDirectory(); + try + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + using (var docView = new TestXhtmlDocView()) + { + CreateReversalIndex("en"); + var spanishIndex = CreateReversalIndex("es"); + var englishConfig = Path.Combine(configDirectory, "en" + DictionaryConfigurationModel.FileExtension); + var spanishConfig = Path.Combine(configDirectory, "es" + DictionaryConfigurationModel.FileExtension); + WriteReversalConfiguration(englishConfig, "English-print", "en"); + WriteReversalConfiguration(spanishConfig, "Spanish", "es"); + docView.SetPropertyTable(m_propertyTable); + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", englishConfig, false); + m_propertyTable.SetProperty(ReversalIndexEntryUi.ReversalIndexGuidProperty, spanishIndex.Guid.ToString(), false); + + // SUT: following a link into a Spanish entry must still pull + // the configuration across (FWR-1105) + string replacement; + var needsRewrite = docView.TryGetReplacementReversalConfiguration("es", configDirectory, out replacement); + + Assert.That(needsRewrite, Is.True); + Assert.That(replacement, Is.EqualTo(spanishConfig)); + } + } + finally + { + Directory.Delete(configDirectory, true); + } + } + + [Test] + public void SilentLayoutChange_SyncingGuidThenValidating_KeepsTheChosenConfiguration() + { + var configDirectory = CreateReversalConfigDirectory(); + try + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + using (var docView = new TestXhtmlDocView()) + { + CreateReversalIndex("en"); + var spanishIndex = CreateReversalIndex("es"); + var englishConfig = Path.Combine(configDirectory, "en" + DictionaryConfigurationModel.FileExtension); + var spanishConfig = Path.Combine(configDirectory, "es" + DictionaryConfigurationModel.FileExtension); + WriteReversalConfiguration(englishConfig, "English-print", "en"); + WriteReversalConfiguration(spanishConfig, "Spanish", "es"); + docView.SetPropertyTable(m_propertyTable); + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", englishConfig, false); + docView.SyncReversalIndexGuidToConfiguration(); + // The Configure dialog saves the Spanish choice with the broadcast + // suppressed, + // so the guid still names the English index at this point + m_propertyTable.SetProperty("ReversalIndexPublicationLayout", spanishConfig, false); + + // SUT: what RefreshAllContent does when the dialog closes + var indexChanged = docView.SyncReversalIndexGuidToConfiguration(); + + Assert.That(indexChanged, Is.True, "The dialog's choice must move the selection to the Spanish index"); + Assert.That(m_propertyTable.GetStringProperty(ReversalIndexEntryUi.ReversalIndexGuidProperty, string.Empty), + Is.EqualTo(spanishIndex.Guid.ToString())); + string replacement; + Assert.That(docView.TryGetReplacementReversalConfiguration("en", configDirectory, out replacement), Is.False, + "The English entry the clerk still holds must not pull the configuration back to English"); + } + } + finally + { + Directory.Delete(configDirectory, true); + } + } + + #endregion + private class TestXhtmlDocView : XhtmlDocView { internal void SetConfigObjectName(string name)