diff --git a/Src/LexText/LexTextDll/AreaListener.cs b/Src/LexText/LexTextDll/AreaListener.cs
index 1fd563f85f..dd2de3e5da 100644
--- a/Src/LexText/LexTextDll/AreaListener.cs
+++ b/Src/LexText/LexTextDll/AreaListener.cs
@@ -28,18 +28,6 @@ public class AreaListener : IxCoreColleague, IDisposable
protected Mediator m_mediator;
protected PropertyTable m_propertyTable;
- ///
- /// Keeps track of how many lists are loaded into List area
- /// memory windowConfiguration XML (including Custom ones).
- ///
- private int m_ctotalLists;
-
- ///
- /// Keeps track of how many Custom lists are loaded into List area
- /// memory windowConfiguration XML.
- ///
- private int m_ccustomLists;
-
#endregion
#region IDisposable & Co. implementation
@@ -152,8 +140,6 @@ public void Init(Mediator mediator, PropertyTable propertyTable, XmlNode configu
m_mediator = mediator;
m_propertyTable = propertyTable;
mediator.AddColleague(this);
- m_ctotalLists = 0;
- m_ccustomLists = 0;
Subscriber.Subscribe(EventConstants.SetInitialContentObject, SetInitialContentObject, m_propertyTable.GetWindow());
Subscriber.Subscribe(EventConstants.SetToolFromName, SetToolFromName, m_propertyTable.GetWindow());
Subscriber.Subscribe(EventConstants.ReloadAreaTools, ReloadAreaTools, m_propertyTable.GetWindow());
@@ -326,56 +312,45 @@ public bool OnDisplayListsToolsList(object parameters, ref UIListDisplayProperti
/// Lists area listener was getting too different from other areas, so I made a
/// separate version of FillList just for the Lists area.
///
- ///
- ///
private bool FillListAreaList(UIListDisplayProperties display)
{
- var customLists = GetListOfOwnerlessLists();
- var fcustomChanged = customLists.Count != m_ccustomLists;
- // Since we're in the 'Lists' area, don't bother refreshing this
- // list, unless the number of Custom lists has changed. This happens
- // whenever someone adds one.
- if (display.List.Count > 0 && !fcustomChanged)
+ var windowConfiguration = m_propertyTable.GetValue("WindowConfiguration");
+ var ownerlessLists = GetListOfOwnerlessLists();
+ var configuredGuids = GetConfiguredOwnerlessListGuids(windowConfiguration);
+ // Only additions are reconciled: deleting a Custom list rebuilds the window it was
+ // deleted in, discarding that window's copy of this configuration.
+ var newLists = ownerlessLists.Where(list => !configuredGuids.Contains(list.Guid)).ToList();
+ // Since we're in the 'Lists' area, don't bother refreshing this list unless a Custom
+ // list has shown up that the window configuration doesn't know about yet.
+ if (display.List.Count > 0 && newLists.Count == 0)
return true;
- // We can get here in the following cases:
- // Case 1: display.List is empty and m_ctotalLists == 0
- // Load 'windowConfiguration' with all Custom lists,
- // Update both list counts,
- // Load 'display' with ALL lists.
- // Case 2: display.List is empty, but m_ctotalLists > 0
- // We MAY have a recent Custom list addition to add.
- // If 'fcustomChanged', load the new Custom list into 'windowConfiguration',
- // and update both list counts,
- // Load 'display' with ALL lists.
- // Case 3: display.List is loaded, but we have a recent Custom list addition to add.
- // Load the new Custom list into 'windowConfiguration',
- // Update both list counts,
- // Only update 'display' with new Custom list.
- // N.B. This may need changing if we allow the user to DELETE Custom lists someday.
- var windowConfiguration = m_propertyTable.GetValue("WindowConfiguration");
- UpdateWinConfig(fcustomChanged, customLists, windowConfiguration);
+ if (newLists.Count > 0)
+ {
+ AddListsToWindowConfig(newLists, windowConfiguration);
+ UpdateMediatorConfig(windowConfiguration);
+ }
+
+ var toolNodes = windowConfiguration.SelectNodes(XWindow.GetToolXPath("lists"));
+ if (toolNodes == null)
+ return true;
- // Now update 'display'
+ // A display that already holds the other lists only wants the ones just added.
+ var wantNewListsOnly = display.List.Count > 0;
+ var newListGuids = new HashSet(newLists.Select(list => list.Guid));
+ var liveGuids = new HashSet(ownerlessLists.Select(list => list.Guid));
var cache = m_propertyTable.GetValue("cache");
var possRepo = cache.ServiceLocator.GetInstance();
- if (display.List.Count > 0)
- {
- var node = windowConfiguration.SelectSingleNode(GetListToolsXPath()).LastChild;
- if (node != null)
- AddToolNodeToDisplay(possRepo, cache, display, node);
- }
- else
+ foreach (XmlNode toolNode in toolNodes)
{
- var nodes = windowConfiguration.SelectNodes(XWindow.GetToolXPath("lists"));
- if (nodes == null)
- {
- return true;
- }
- foreach (XmlNode node in nodes)
- {
- AddToolNodeToDisplay(possRepo, cache, display, node);
- }
+ var listGuid = GetOwnerlessListGuidFromToolNode(toolNode);
+ // A Custom list deleted in another main window leaves its tool in this window's
+ // configuration, where the guid no longer resolves to a list.
+ if (listGuid != Guid.Empty && !liveGuids.Contains(listGuid))
+ continue;
+ if (wantNewListsOnly && !newListGuids.Contains(listGuid))
+ continue;
+ AddToolNodeToDisplay(possRepo, cache, display, toolNode);
}
return true;
}
@@ -534,41 +509,38 @@ private static XmlNode FindClerkNode(XmlNode toolNode, string clerkId)
return null;
}
- private void UpdateWinConfig(bool fcustomChanged, List customLists, XmlNode windowConfig)
- {
- // See caller FillListAreaList() for description of Case 1-3
- if (m_ctotalLists == 0) // Case 1
- LoadAllCustomLists(customLists, windowConfig);
- else // Case 2 and 3
- if (fcustomChanged) // Case 3 is automatically true
- AddACustomList(customLists[customLists.Count - 1], windowConfig);
- }
-
- private void AddACustomList(ICmPossibilityList customList, XmlNode windowConfig)
+ ///
+ /// Gets the guids of the ownerless lists that the 'lists' area of the given window
+ /// configuration already has a tool for.
+ ///
+ private static HashSet GetConfiguredOwnerlessListGuids(XmlNode windowConfig)
{
- // Add 'customList' to windowConfig
- AddListsToWindowConfig(new List {customList}, windowConfig);
-
- // We have to update this because other things besides 'tools' need to get set.
- UpdateMediatorConfig(windowConfig);
-
- m_ccustomLists++;
- m_ctotalLists++;
+ var configuredGuids = new HashSet();
+ var toolNodes = windowConfig.SelectNodes(XWindow.GetToolXPath("lists"));
+ if (toolNodes == null)
+ return configuredGuids;
+ foreach (XmlNode toolNode in toolNodes)
+ {
+ var listGuid = GetOwnerlessListGuidFromToolNode(toolNode);
+ if (listGuid != Guid.Empty)
+ configuredGuids.Add(listGuid);
+ }
+ return configuredGuids;
}
- private void LoadAllCustomLists(List customLists, XmlNode windowConfig)
+ ///
+ /// Gets the guid of the ownerless list that the given tool edits, or Guid.Empty if the
+ /// tool edits an owned list or isn't wired to a list at all.
+ ///
+ private static Guid GetOwnerlessListGuidFromToolNode(XmlNode toolNode)
{
-
- AddListsToWindowConfig(customLists, windowConfig);
-
- // We have to update this because other things besides 'tools' need to get set.
- UpdateMediatorConfig(windowConfig);
-
- var nodes = windowConfig.SelectNodes(XWindow.GetToolXPath("lists"));
- if (nodes != null)
- m_ctotalLists = nodes.Count;
- m_ccustomLists = customLists.Count;
- m_ctotalLists += m_ccustomLists;
+ var recordListNode = GetClerkRecordListNodeFromToolNode(toolNode);
+ if (recordListNode == null || XmlUtils.GetAttributeValue(recordListNode, "owner") != "unowned")
+ return Guid.Empty;
+ // An unowned recordList carries the list's guid as its property.
+ Guid listGuid;
+ return Guid.TryParse(XmlUtils.GetAttributeValue(recordListNode, "property"), out listGuid)
+ ? listGuid : Guid.Empty;
}
private void UpdateMediatorConfig(XmlNode windowConfig)
diff --git a/Src/LexText/LexTextDll/LexTextDllTests/AreaListenerTests.cs b/Src/LexText/LexTextDll/LexTextDllTests/AreaListenerTests.cs
index 484649e487..6a6da8d10a 100644
--- a/Src/LexText/LexTextDll/LexTextDllTests/AreaListenerTests.cs
+++ b/Src/LexText/LexTextDll/LexTextDllTests/AreaListenerTests.cs
@@ -8,6 +8,7 @@
//
//
using System.Collections.Generic;
+using System.Linq;
using System.Xml;
using NUnit.Framework;
using SIL.LCModel;
@@ -29,6 +30,11 @@ public class AreaListenerTests : MemoryOnlyBackendProviderRestoredForEachTestTes
{
#region Member Data
+ private const string ClerkXPath = "//item[@value='lists']/parameters/clerks/clerk";
+ private const string CommandXPath = "//commands/command";
+ private const string ContextMenuXPath = "//contextMenus/menu";
+ private const string ToolXPath = "//item[@value='lists']/parameters/tools/tool";
+
///
/// For testing.
///
@@ -146,6 +152,29 @@ private static XmlNode SetupWindowConfigWithListTool(string clerkId, string tool
return fakeWindowConfig.DocumentElement;
}
+ private void UseTestWindowConfiguration()
+ {
+ m_propertyTable.SetProperty("WindowConfiguration", m_testWindowConfig, true);
+ m_propertyTable.SetPropertyPersistence("WindowConfiguration", false);
+ }
+
+ private ICmPossibilityList CreateUnownedList(string name)
+ {
+ return Cache.ServiceLocator.GetInstance()
+ .CreateUnowned(name, WritingSystemServices.kwsAnals);
+ }
+
+ ///
+ /// Refreshes the Lists area sidebar the way xCore does and returns the tool name of every
+ /// entry the refresh left in the display, in display order.
+ ///
+ private List RefreshListsToolsDisplay()
+ {
+ var display = new UIListDisplayProperties(new XCore.List(null));
+ m_listener.OnDisplayListsToolsList(null, ref display);
+ return display.List.Cast().Select(item => item.value).ToList();
+ }
+
#endregion
///--------------------------------------------------------------------------------------
@@ -158,24 +187,18 @@ public void AddListToXmlConfig()
{
// Setup
var node = m_testWindowConfig;
- const string clerkXPath = "//item[@value='lists']/parameters/clerks/clerk";
- const string commandXPath = "//commands/command";
- const string contextXPath = "//contextMenus/menu";
- const string toolXPath = "//item[@value='lists']/parameters/tools/tool";
- //var fakeUIDisplay = new UIListDisplayProperties(new XCore.List(node.SelectSingleNode(toolXPath), null));
+ //var fakeUIDisplay = new UIListDisplayProperties(new XCore.List(node.SelectSingleNode(ToolXPath), null));
//var cdispNodesBefore = fakeUIDisplay.List.Count;
- var contextNodes = node.SelectNodes(contextXPath);
+ var contextNodes = node.SelectNodes(ContextMenuXPath);
var ccontextNodesBefore = contextNodes == null ? 0 : contextNodes.Count;
- var commandNodes = node.SelectNodes(commandXPath);
+ var commandNodes = node.SelectNodes(CommandXPath);
var ccommandNodesBefore = commandNodes == null ? 0 : commandNodes.Count;
- var clerkNodes = node.SelectNodes(clerkXPath);
+ var clerkNodes = node.SelectNodes(ClerkXPath);
var cclerkNodesBefore = clerkNodes == null ? 0 : clerkNodes.Count;
- var toolNodes = node.SelectNodes(toolXPath);
+ var toolNodes = node.SelectNodes(ToolXPath);
var ctoolNodesBefore = toolNodes == null ? 0 : toolNodes.Count;
- const string listName = "testList1";
- var ws = WritingSystemServices.kwsAnals;
- var testList = Cache.ServiceLocator.GetInstance().CreateUnowned(listName, ws);
+ var testList = CreateUnownedList("testList1");
// SUT
m_listener.AddListsToWindowConfig(new List { testList }, node);
@@ -183,16 +206,101 @@ public void AddListToXmlConfig()
// Verify
// The above routine no longer handles display nodes
//Assert.That(fakeUIDisplay.List.Count, Is.EqualTo(cdispNodesBefore + 1), "Didn't add a display node.");
- var ctoolNodesAfter = node.SelectNodes(toolXPath).Count;
+ var ctoolNodesAfter = node.SelectNodes(ToolXPath).Count;
Assert.That(ctoolNodesAfter, Is.EqualTo(ctoolNodesBefore + 1), "Didn't add a tool node.");
- var cclerkNodesAfter = node.SelectNodes(clerkXPath).Count;
+ var cclerkNodesAfter = node.SelectNodes(ClerkXPath).Count;
Assert.That(cclerkNodesAfter, Is.EqualTo(cclerkNodesBefore + 1), "Didn't add a clerk node.");
- var ccommandNodesAfter = node.SelectNodes(commandXPath).Count;
+ var ccommandNodesAfter = node.SelectNodes(CommandXPath).Count;
Assert.That(ccommandNodesAfter, Is.EqualTo(ccommandNodesBefore + 1), "Didn't add a command node.");
- var ccontextNodesAfter = node.SelectNodes(contextXPath).Count;
+ var ccontextNodesAfter = node.SelectNodes(ContextMenuXPath).Count;
Assert.That(ccontextNodesAfter, Is.EqualTo(ccontextNodesBefore + 1), "Didn't add a context menu node.");
}
+ ///--------------------------------------------------------------------------------------
+ ///
+ /// Every ownerless list created since the previous refresh must reach the Lists area, not
+ /// just one of them; a single session can create several.
+ ///
+ ///--------------------------------------------------------------------------------------
+ [Test]
+ public void FillListAreaList_AddsEveryListCreatedSinceLastRefresh()
+ {
+ UseTestWindowConfiguration();
+ CreateUnownedList("Original List");
+ var afterFirstRefresh = RefreshListsToolsDisplay();
+ Assert.That(afterFirstRefresh, Does.Contain("OriginalListEdit"));
+
+ CreateUnownedList("Second List");
+ CreateUnownedList("Third List");
+
+ // SUT
+ var afterSecondRefresh = RefreshListsToolsDisplay();
+
+ Assert.That(afterSecondRefresh.Except(afterFirstRefresh),
+ Is.EquivalentTo(new[] { "SecondListEdit", "ThirdListEdit" }), "Missed a new list.");
+ Assert.That(afterSecondRefresh, Is.Unique, "Showed a list more than once.");
+ }
+
+ ///--------------------------------------------------------------------------------------
+ ///
+ /// A refresh that finds no new ownerless list must leave the window configuration and the
+ /// mediator command set alone. Adding a list a second time appends duplicate tool, clerk
+ /// and context menu nodes, and throws on its already registered command id.
+ ///
+ ///--------------------------------------------------------------------------------------
+ [Test]
+ public void FillListAreaList_RepeatedRefresh_DoesNotDuplicateConfigNodes()
+ {
+ UseTestWindowConfiguration();
+ CreateUnownedList("Original List");
+ RefreshListsToolsDisplay();
+ CreateUnownedList("Second List");
+ CreateUnownedList("Third List");
+ var afterListsAdded = RefreshListsToolsDisplay();
+ var ctoolNodes = m_testWindowConfig.SelectNodes(ToolXPath).Count;
+ var cclerkNodes = m_testWindowConfig.SelectNodes(ClerkXPath).Count;
+ var ccommandNodes = m_testWindowConfig.SelectNodes(CommandXPath).Count;
+ var ccontextNodes = m_testWindowConfig.SelectNodes(ContextMenuXPath).Count;
+
+ // SUT: a refresh with nothing new to pick up
+ var afterIdleRefresh = RefreshListsToolsDisplay();
+
+ Assert.That(afterIdleRefresh, Is.EquivalentTo(afterListsAdded));
+ Assert.That(m_testWindowConfig.SelectNodes(ToolXPath).Count,
+ Is.EqualTo(ctoolNodes), "Duplicated a tool node.");
+ Assert.That(m_testWindowConfig.SelectNodes(ClerkXPath).Count,
+ Is.EqualTo(cclerkNodes), "Duplicated a clerk node.");
+ Assert.That(m_testWindowConfig.SelectNodes(CommandXPath).Count,
+ Is.EqualTo(ccommandNodes), "Duplicated a command node.");
+ Assert.That(m_testWindowConfig.SelectNodes(ContextMenuXPath).Count,
+ Is.EqualTo(ccontextNodes), "Duplicated a context menu node.");
+ }
+
+ ///--------------------------------------------------------------------------------------
+ ///
+ /// Deleting a Custom list rebuilds only the window it was deleted in, so any other main
+ /// window keeps a tool whose list is gone. Refreshing there must drop that tool rather
+ /// than fail to resolve its guid.
+ ///
+ ///--------------------------------------------------------------------------------------
+ [Test]
+ public void FillListAreaList_ListDeletedInAnotherWindow_DropsItsTool()
+ {
+ UseTestWindowConfiguration();
+ var doomedList = CreateUnownedList("Doomed List");
+ CreateUnownedList("Surviving List");
+ var afterFirstRefresh = RefreshListsToolsDisplay();
+ Assert.That(afterFirstRefresh, Does.Contain("DoomedListEdit"));
+
+ doomedList.Delete();
+
+ // SUT
+ var afterDeletion = RefreshListsToolsDisplay();
+
+ Assert.That(afterDeletion, Does.Not.Contain("DoomedListEdit"));
+ Assert.That(afterDeletion, Does.Contain("SurvivingListEdit"), "Dropped the wrong tool.");
+ }
+
///--------------------------------------------------------------------------------------
///
/// Publishing EventConstants.GetToolForList for a list that is wired into the window