LT-22702: Escape custom list range ids at the point they are written

Src/LexText/LexTextControls/LexTextControlsTests/LiftExportTests.cs

@@ -493,6 +493,12 @@ public void CreateMockCache()
 			m_mapPartsOfSpeech.Clear();
 			m_mapAcademicDomains.Clear();
 			m_mapPublications.Clear();
+			// NUnit shares one fixture instance across the tests, so these outlive the cache they index.
+			m_customFieldEntryIds.Clear();
+			m_customFieldSenseIds.Clear();
+			m_customFieldAllomorphsIds.Clear();
+			m_customFieldExampleSentencesIds.Clear();
+			m_customListsGuids.Clear();
 			var mockProjectName = "xxyyzProjectFolderForLIFTTest";
 			MockProjectFolder = Path.Combine(Path.GetTempPath(), mockProjectName);
 			var mockProjectPath = Path.Combine(MockProjectFolder, mockProjectName + ".fwdata");
@@ -1135,6 +1141,81 @@ public void LiftExport()
 			VerifyExportRanges(xdocRangeFile);
 		}
 
+		///--------------------------------------------------------------------------------------
+		/// <summary>
+		/// A custom list name reaches the ranges file as a range id, which is an attribute, so a name
+		/// holding a quotation mark has to be escaped by the attribute rules for the document to parse.
+		/// </summary>
+		///--------------------------------------------------------------------------------------
+		[Test]
+		public void LiftExportRanges_CustomListRangeIdWithQuoteIsEscapedForAnAttribute()
+		{
+			const string ksListName = "So-called \"words\"";
+			XmlDocument xdocLift;
+			XmlDocument xdocRangeFile;
+			var customList = ExportWithCustomList(ksListName, out xdocLift, out xdocRangeFile);
+
+			var range = xdocRangeFile.SelectSingleNode(string.Format("//range[@guid='{0}']", customList.Guid));
+			Assert.That(range, Is.Not.Null, "the custom list must be written as a range");
+			Assert.That(range.Attributes["id"].Value, Is.EqualTo(ksListName),
+				"the range id must read back as the name stored");
+		}
+
+		///--------------------------------------------------------------------------------------
+		/// <summary>
+		/// A custom list name is written as a range id in both the .lift header and the ranges file,
+		/// escaped once in each, so that the two files name the range the same way.
+		/// </summary>
+		///--------------------------------------------------------------------------------------
+		[Test]
+		public void LiftExportRanges_CustomListRangeIdWithAmpersandIsEscapedExactlyOnce()
+		{
+			const string ksListName = "Birds & Beasts";
+			XmlDocument xdocLift;
+			XmlDocument xdocRangeFile;
+			var customList = ExportWithCustomList(ksListName, out xdocLift, out xdocRangeFile);
+
+			var rangesRange = xdocRangeFile.SelectSingleNode(string.Format("//range[@guid='{0}']", customList.Guid));
+			Assert.That(rangesRange, Is.Not.Null);
+			Assert.That(rangesRange.Attributes["id"].Value, Is.EqualTo(ksListName),
+				"one level of escaping, undone by the parser, must give back the name stored");
+			var headerIds = xdocLift.SelectNodes("//header/ranges/range/@id");
+			Assert.That(headerIds, Is.Not.Null);
+			Assert.That(headerIds.Cast<XmlNode>().Any(id => id.Value == ksListName),
+				"the .lift header must name the range the same way the ranges file does");
+		}
+
+		/// <summary>
+		/// Exports both files from one exporter, so that the custom list is discovered by the .lift
+		/// pass and is therefore written by the ranges pass.
+		/// </summary>
+		private ICmPossibilityList ExportWithCustomList(string listName, out XmlDocument xdocLift, out XmlDocument xdocRangeFile)
+		{
+			ICmPossibilityList customList = null;
+			NonUndoableUnitOfWorkHelper.Do(m_cache.ActionHandlerAccessor, () =>
+			{
+				customList = AddCustomList(listName);
+				MakeCustomField("CustomFieldForCustomList", LexEntryTags.kClassId,
+					WritingSystemServices.kwsAnal, CustomFieldType.ListRefAtomic, customList.Guid);
+			});
+			var exporter = new LiftExporter(m_cache);
+			xdocLift = new XmlDocument();
+			using (var w = new StringWriter())
+			{
+				// SUT: the .lift pass populates the map the ranges pass writes from.
+				exporter.ExportLift(w, LiftFolder);
+				xdocLift.LoadXml(w.ToString());
+			}
+			xdocRangeFile = new XmlDocument();
+			using (var w = new StringWriter())
+			{
+				// SUT
+				exporter.ExportLiftRanges(w);
+				xdocRangeFile.LoadXml(w.ToString());
+			}
+			return customList;
+		}
+
 		/// <summary>
 		/// LT-15467 documents a Flex to WeSay S/R which had pronunciation audio files multiplying like bunny rabbits.
 		/// Make sure the export doesn't make new files when two different references point to the same file.

Src/LexText/LexTextControls/LiftExporter.cs

@@ -56,8 +56,14 @@ public class LiftExporter
 		private readonly int m_wsBestVernAnal;
 		/// <summary>
 		/// This contains the possibility lists that are custom or that are referenced by a custom field.
+		/// Range names are held unescaped, so that each is escaped by the rules of the context it is
+		/// written into.
 		/// </summary>
 		private Dictionary<Guid, string> m_CmPossListsReferencedOrCustom = new Dictionary<Guid, string>();
+		/// <summary>
+		/// Maps a possibility list to the name of its LIFT range. Range names are held unescaped, so
+		/// that each is escaped by the rules of the context it is written into.
+		/// </summary>
 		private Dictionary<Guid, string> m_ListsGuidToRangeName = new Dictionary<Guid, string>();
 		private readonly ICmPossibilityListRepository m_repoCmPossibilityLists;
 		private readonly ISilDataAccessManaged m_sda;
@@ -1979,10 +1985,11 @@ private void WritePossibilityListAsRange(TextWriter w, string rangeId, ICmPossib
 		{
 			if (list.PossibilitiesOS.Count == 0)
 				return;
+			var sRangeId = MakeSafeAndNormalizedAttribute(rangeId);
 			if (String.IsNullOrEmpty(guid))					//only output the guid for custom lists
-				w.WriteLine("<range id=\"{0}\">", rangeId);
+				w.WriteLine("<range id=\"{0}\">", sRangeId);
 			else
-				w.WriteLine("<range id=\"{0}\" guid=\"{1}\">", rangeId, MakeSafeAndNormalizedAttribute(guid));
+				w.WriteLine("<range id=\"{0}\" guid=\"{1}\">", sRangeId, MakeSafeAndNormalizedAttribute(guid));
 			foreach (var poss in list.ReallyReallyAllPossibilities)
 			{
 				var liftId = poss.Name.BestAnalysisVernacularAlternative.Text;
@@ -2348,7 +2355,7 @@ private Dictionary<Guid, string> GetCustomListsAndReferencedLists(IEnumerable<IL
 						{
 							var rangeName = RangeNames.GetRangeNameForLiftExport(m_mdc, possList);
 							if (!listsToBeExported.ContainsKey(possList.Guid))
-								listsToBeExported.Add(possList.Guid, MakeSafeAndNormalizedXml(rangeName));
+								listsToBeExported.Add(possList.Guid, rangeName);
 						}
 						break;
 					default:
@@ -2367,7 +2374,7 @@ private void GetCustomListsNotYetAdded(ICmPossibilityListRepository possListRepo
 				if (list.Owner == null && !listsToBeExported.ContainsKey(list.Guid))
 				{
 					var rangeName = RangeNames.GetRangeNameForLiftExport(m_mdc, list);
-					listsToBeExported[list.Guid] = MakeSafeAndNormalizedXml(rangeName);
+					listsToBeExported[list.Guid] = rangeName;
 				}
 			}
 		}
@@ -2440,7 +2447,7 @@ private void AddPossListRefdByField(int possHvo, Dictionary<Guid, string> cmPoss
 				var rangeName = RangeNames.GetRangeNameForLiftExport(m_mdc, possList);
 				if (!cmPossibilityListsReferencedByFields.ContainsKey(possListGuid))
 				{
-					cmPossibilityListsReferencedByFields.Add(possListGuid, MakeSafeAndNormalizedXml(rangeName));
+					cmPossibilityListsReferencedByFields.Add(possListGuid, rangeName);
 				}
 			}
 		}
@@ -2463,7 +2470,7 @@ private void MapCmPossibilityListGuidsToLiftRangeNames(Dictionary<Guid, string>
 			//by a custom field and which are not already included yet.
 			foreach (var list in cmPossibilityListsReferencedByFields)
 			{
-				m_ListsGuidToRangeName.Add(list.Key, MakeSafeAndNormalizedXml(list.Value));
+				m_ListsGuidToRangeName.Add(list.Key, list.Value);
 			}
 		}