From 9a87fdce8244c127db9065e17bbe46eec34881d8 Mon Sep 17 00:00:00 2001 From: mark-sil <83427558+mark-sil@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:16:41 -0400 Subject: [PATCH] Replace XhtmlDocView's SelectedPublication broadcasts with direct calls XhtmlDocView set SelectedPublication with the broadcast flag at two sites whose only listener is the view itself: RefreshAllContent, which F5's MasterRefresh and the DictionaryConfigured event reach, when the current configuration excludes the selected publication, and OnShowAllEntries, the handler of the ShowAllEntries command. The xCore Mediator delivered those broadcasts after the caller had returned, so RefreshAllContent loaded the configuration it had computed and the deferred handler then asked for the configuration valid for the new publication. A second load followed whenever that was another file or the publication fell back to All Entries, and the right configuration ended up loaded only because the deferred load came last. Both sites now store SelectedPublication without a broadcast and do the handler's work directly, in place of the deferred load. RefreshAllContent loads once: the configuration valid for the publication it moved to, or the current configuration when the publication stays. Show All Entries loads once, and choosing it while All Entries is already selected changes nothing, as the PropertyTable's change-only broadcast made it before. The OnPropertyChanged case for SelectedPublication stays, because the publication menus (ChoiceGroup) and the window's default properties still set the name with a broadcast. The value stays in the PropertyTable because GetCurrentPublication, XhtmlRecordDocView and ConfiguredLcmGenerator read it while the content is generated. The DictionaryPublicationLayout and ReversalIndexPublicationLayout handler and m_updateContentLater are unchanged. Co-Authored-By: Claude Opus 5.5 --- Src/xWorks/XhtmlDocView.cs | 48 +++++- Src/xWorks/xWorksTests/XhtmlDocViewTests.cs | 177 ++++++++++++++++++++ 2 files changed, 216 insertions(+), 9 deletions(-) diff --git a/Src/xWorks/XhtmlDocView.cs b/Src/xWorks/XhtmlDocView.cs index db86a52338..f3c1b8848e 100644 --- a/Src/xWorks/XhtmlDocView.cs +++ b/Src/xWorks/XhtmlDocView.cs @@ -1123,8 +1123,7 @@ public void OnPropertyChanged(string name) switch (name) { case "SelectedPublication": - var validConfiguration = SetCurrentDictionaryPublicationLayout(); - UpdateContent(validConfiguration); + ShowSelectedPublication(); break; case "DictionaryPublicationLayout": case "ReversalIndexPublicationLayout": @@ -1422,15 +1421,39 @@ private void RefreshAllContent(object _) // The Configure dialog sets the layout silently, so sync the guid. SyncReversalIndexGuidToConfiguration(); } + // Force a refresh. + m_loadedConfig = null; + ShowConfiguration(currentConfig); + } + + /// + /// Loads a configuration. When it excludes the selected publication, the publication + /// first moves, without broadcasting, to one the configuration includes, or to All + /// Entries when it includes none, and the load is then of the configuration valid for + /// that publication. All Entries never moves. + /// + internal void ShowConfiguration(string configuration) + { var currentPublication = GetCurrentPublication(); - var validPublication = GetValidPublicationForConfiguration(currentConfig) ?? xWorksStrings.AllEntriesPublication; + var validPublication = GetValidPublicationForConfiguration(configuration) ?? xWorksStrings.AllEntriesPublication; if (currentPublication != xWorksStrings.AllEntriesPublication && currentPublication != validPublication) { - m_propertyTable.SetProperty("SelectedPublication", validPublication, true); + m_propertyTable.SetProperty("SelectedPublication", validPublication, false); + ShowSelectedPublication(); } - // Force a refresh. - m_loadedConfig = null; - UpdateContent(currentConfig); + else + { + UpdateContent(configuration); + } + } + + /// + /// Stores the configuration valid for the selected publication, without broadcasting, + /// and loads it. + /// + private void ShowSelectedPublication() + { + UpdateContent(SetCurrentDictionaryPublicationLayout()); } public bool RefreshDisplay() @@ -1603,13 +1626,20 @@ public virtual bool OnDisplayFindAndReplaceText(object commandObject, ref UIItem return true; //we've handled this } + /// + /// Mediator handler for the Show All Entries command: selects All Entries and shows it. + /// Choosing it again changes nothing. + /// public bool OnShowAllEntries(object args) { - m_propertyTable.SetProperty("SelectedPublication", xWorksStrings.AllEntriesPublication, true); + if (GetCurrentPublication() == xWorksStrings.AllEntriesPublication) + return true; + m_propertyTable.SetProperty("SelectedPublication", xWorksStrings.AllEntriesPublication, false); + ShowSelectedPublication(); return true; } - private void UpdateContent(string configurationFile, bool allOnOnePage = false) + protected virtual void UpdateContent(string configurationFile, bool allOnOnePage = false) { SetInfoBarText(); var htmlErrorMessage = xWorksStrings.ksErrorDisplayingPublication; diff --git a/Src/xWorks/xWorksTests/XhtmlDocViewTests.cs b/Src/xWorks/xWorksTests/XhtmlDocViewTests.cs index 2019234ac0..69802c06e4 100644 --- a/Src/xWorks/xWorksTests/XhtmlDocViewTests.cs +++ b/Src/xWorks/xWorksTests/XhtmlDocViewTests.cs @@ -655,8 +655,185 @@ public void SilentLayoutChange_SyncingGuidThenValidating_KeepsTheChosenConfigura #endregion + #region Direct publication selection + + [Test] + public void OnShowAllEntries_SelectsAllEntriesAndLoadsTheCurrentConfigurationOnce() + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + { + CreatePublication("TestPub"); + using (var scenario = new PublicationScenario(Cache, "TestPub", "TestPub")) + { + // SUT + scenario.View.OnShowAllEntries(null); + + Assert.That(scenario.SelectedPublication, Is.EqualTo(xWorksStrings.AllEntriesPublication)); + Assert.That(scenario.View.LoadedConfigurations, Is.EqualTo(new[] { scenario.ConfigurationPath })); + Assert.That(scenario.Mediator.JobItems, Is.Zero, "the publication must change without a broadcast"); + } + } + } + + [Test] + public void OnShowAllEntries_WhenAllEntriesIsSelected_LoadsNothing() + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + { + CreatePublication("TestPub"); + using (var scenario = new PublicationScenario(Cache, "TestPub", xWorksStrings.AllEntriesPublication)) + { + // SUT + scenario.View.OnShowAllEntries(null); + + Assert.That(scenario.View.LoadedConfigurations, Is.Empty); + Assert.That(scenario.Mediator.JobItems, Is.Zero); + } + } + } + + [Test] + public void ShowConfiguration_ExcludedPublicationSelected_MovesToAnIncludedOneAndLoadsOnce() + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + { + CreatePublication("TestPub"); + CreatePublication("OtherPub"); + using (var scenario = new PublicationScenario(Cache, "TestPub", "OtherPub")) + { + // SUT + scenario.View.ShowConfiguration(scenario.ConfigurationPath); + + Assert.That(scenario.SelectedPublication, Is.EqualTo("TestPub")); + Assert.That(scenario.View.LoadedConfigurations, Is.EqualTo(new[] { scenario.ConfigurationPath }), + "one load, of the configuration valid for the publication moved to"); + Assert.That(scenario.Mediator.JobItems, Is.Zero, "the publication must change without a broadcast"); + } + } + } + + [Test] + public void ShowConfiguration_ValidConfigurationIsAnotherFile_LoadsOnlyTheValidOne() + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + { + CreatePublication("TestPub"); + CreatePublication("OtherPub"); + using (var scenario = new PublicationScenario(Cache, "TestPub", "OtherPub")) + using (var outsideConfig = TempFile.WithFilename(Path.Combine(Path.GetTempPath(), + "OutsideTheProject" + DictionaryConfigurationModel.FileExtension))) + { + // Outside the project folder, so the publication's valid configuration is the + // project one, not this file + File.WriteAllText(outsideConfig.Path, ConfigurationTemplate.Replace("", + "TestPub")); + scenario.PropertyTable.SetProperty("DictionaryPublicationLayout", outsideConfig.Path, false); + + // SUT + scenario.View.ShowConfiguration(outsideConfig.Path); + + Assert.That(scenario.SelectedPublication, Is.EqualTo("TestPub")); + Assert.That(scenario.View.LoadedConfigurations, Is.EqualTo(new[] { scenario.ConfigurationPath }), + "the configuration passed in must not be loaded when the publication moved"); + Assert.That(scenario.PropertyTable.GetStringProperty("DictionaryPublicationLayout", null), + Is.EqualTo(scenario.ConfigurationPath)); + } + } + } + + [Test] + public void ShowConfiguration_AllEntriesSelected_KeepsAllEntriesAndLoadsOnce() + { + using (new UndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor, "doit", "undoit")) + { + CreatePublication("TestPub"); + using (var scenario = new PublicationScenario(Cache, "TestPub", xWorksStrings.AllEntriesPublication)) + { + // SUT + scenario.View.ShowConfiguration(scenario.ConfigurationPath); + + Assert.That(scenario.SelectedPublication, Is.EqualTo(xWorksStrings.AllEntriesPublication)); + Assert.That(scenario.View.LoadedConfigurations, Is.EqualTo(new[] { scenario.ConfigurationPath })); + } + } + } + + private void CreatePublication(string name) + { + var publication = Cache.ServiceLocator.GetInstance().Create(); + var enId = Cache.WritingSystemFactory.GetWsFromStr("en"); + Cache.LangProject.LexDbOA.PublicationTypesOA.PossibilitiesOS.Add(publication); + publication.Name.set_String(enId, TsStringUtils.MakeString(name, enId)); + } + + /// + /// A colleague registered so the Mediator queues broadcasts: with no colleague it + /// discards them before queueing, and the job count would prove nothing. + /// + private sealed class ListeningColleague : IxCoreColleague + { + public void Init(Mediator mediator, PropertyTable propertyTable, XmlNode configurationParameters) { } + public IxCoreColleague[] GetMessageTargets() => new IxCoreColleague[] { this }; + public bool ShouldNotCall => false; + public int Priority => (int)ColleaguePriority.High; + } + + /// + /// A Dictionary-tool view on its own Mediator and PropertyTable, whose current + /// configuration is a user configuration listing only the publication named at + /// creation. Disposing it removes that configuration file. + /// + private sealed class PublicationScenario : IDisposable + { + public Mediator Mediator { get; } + public PropertyTable PropertyTable { get; } + public TestXhtmlDocView View { get; } + public string ConfigurationPath => m_configuration.Path; + public string SelectedPublication => PropertyTable.GetStringProperty("SelectedPublication", null); + private readonly TempFile m_configuration; + + public PublicationScenario(LcmCache cache, string publicationInConfiguration, string selectedPublication) + { + Mediator = new Mediator(); + Mediator.AddColleague(new ListeningColleague()); + PropertyTable = new PropertyTable(Mediator); + PropertyTable.SetProperty("cache", cache, false); + var configDirectory = Path.Combine(LcmFileHelper.GetConfigSettingsDir(cache.ProjectId.ProjectFolder), "Dictionary"); + Directory.CreateDirectory(configDirectory); + m_configuration = TempFile.WithFilename(Path.Combine(configDirectory, + "OnlyOnePublication" + DictionaryConfigurationModel.FileExtension)); + File.WriteAllText(m_configuration.Path, ConfigurationTemplate.Replace("", + "" + publicationInConfiguration + "")); + PropertyTable.SetProperty("currentContentControl", "lexiconDictionary", false); + PropertyTable.SetProperty("DictionaryPublicationLayout", m_configuration.Path, false); + PropertyTable.SetProperty("SelectedPublication", selectedPublication, false); + View = new TestXhtmlDocView(); + View.SetConfigObjectName("Dictionary"); + View.SetMediator(Mediator); + View.SetPropertyTable(PropertyTable); + } + + public void Dispose() + { + View.Dispose(); + m_configuration.Dispose(); + PropertyTable.Dispose(); + Mediator.Dispose(); + } + } + + #endregion + private class TestXhtmlDocView : XhtmlDocView { + /// The configurations handed to UpdateContent, in call order. + internal readonly List LoadedConfigurations = new List(); + + protected override void UpdateContent(string configurationFile, bool allOnOnePage = false) + { + LoadedConfigurations.Add(configurationFile); + } + internal void SetConfigObjectName(string name) { m_configObjectName = name;