Fix Issue 7100

Clean up some issues with  ConversionLab

Fix Issue 7100.

Clean up some issues with the use of ConversionLab.

5 files changed, 60 insertions, 20 deletions

diff --git a/src/wix/WixToolset.Converters/ConversionLab.cs b/src/wix/WixToolset.Converters/ConversionLab.cs
index 903071f8..727ff1b1 100644
--- a/src/wix/WixToolset.Converters/ConversionLab.cs
+++ b/src/wix/WixToolset.Converters/ConversionLab.cs
@@ -40,21 +40,58 @@ namespace WixToolset.Converters
             }
         }
 
-        public void RemoveTargetElement()
+        public void InsertElementBeforeTargetElement(XElement newElement)
+        {
+            var index = this.index - 1;
+
+            if (0 <= index
+         && this.siblingNodes[index] is XText leadingText
+         && String.IsNullOrWhiteSpace(leadingText.Value))
+            {
+                this.siblingNodes.Insert(index, newElement);
+                this.siblingNodes.Insert(index, new XText(leadingText.Value));
+                this.index += 2;
+            }
+        }
+
+        private bool IsUniqueElement(XElement newElement)
+        {
+            foreach (XNode node in this.siblingNodes)
+            {
+                if (node is XElement element)
+                {
+                    if (element.Name == newElement.Name)
+                    {
+                        return false;
+                    }
+                }
+            }
+
+            return true;
+        }
+
+        public void InsertUniqueElementBeforeTargetElement(XElement newElement)
         {
-            if (this.index + 1 < this.siblingNodes.Count
-             && this.siblingNodes[this.index + 1] is XText trailingText
-             && String.IsNullOrWhiteSpace(trailingText.Value))
+            if (this.IsUniqueElement(newElement))
             {
-                this.siblingNodes.RemoveAt(this.index + 1);
+                this.InsertElementBeforeTargetElement(newElement);
             }
+        }
+
+        public void RemoveTargetElement()
+        {
             this.siblingNodes.RemoveAt(this.index);
-            if (0 < this.index
-             && this.siblingNodes[this.index - 1] is XText leadingText
+
+            var index = this.index - 1;
+
+            if (0 <= index
+             && this.siblingNodes[index] is XText leadingText
              && String.IsNullOrWhiteSpace(leadingText.Value))
             {
-                this.siblingNodes.RemoveAt(this.index - 1);
+                this.siblingNodes.RemoveAt(index);
             }
+
+            this.RemoveOrphanTextNodes();
         }
 
         public void ReplaceTargetElement(XElement replacement)
diff --git a/src/wix/WixToolset.Converters/WixConverter.cs b/src/wix/WixToolset.Converters/WixConverter.cs
index 7fc6314a..dc4a0bfa 100644
--- a/src/wix/WixToolset.Converters/WixConverter.cs
+++ b/src/wix/WixToolset.Converters/WixConverter.cs
@@ -563,9 +563,7 @@ namespace WixToolset.Converters
                 else if (node is XElement element)
                 {
                     this.ConvertElement(element);
-
                     var before = element.Nodes().ToList();
-
                     this.ConvertNodes(before, level + 1);
 
                     // If any nodes were added during the processing of the children,
@@ -1066,7 +1064,7 @@ namespace WixToolset.Converters
                     {
                         xCondition.Remove();
                         element.Add(new XAttribute("Condition", text));
-                        lab.RemoveOrphanTextNodes();
+                        lab. RemoveOrphanTextNodes();
                         lab.AddCommentsAsSiblings(comments);
                     }
                 }
@@ -1280,6 +1278,7 @@ namespace WixToolset.Converters
                     using (var lab = new ConversionLab(element))
                     {
                         element.Add(new XAttribute("Condition", text));
+                        lab.RemoveOrphanTextNodes();
                         lab.AddCommentsAsSiblings(comments);
                     }
                 }
@@ -1610,13 +1609,15 @@ namespace WixToolset.Converters
             if (!String.IsNullOrEmpty(newElementName)
                 && this.OnInformation(ConverterTestType.ReferencesReplaced, element, "UI, custom action, and property reference {0} has been replaced with strongly-typed element.", id))
             {
-                this.XRoot.SetAttributeValue(XNamespace.Xmlns + newNamespaceName, newNamespace.NamespaceName);
-
-                element.AddBeforeSelf(new XElement(newNamespace + newElementName));
-
-                if (replace)
+                using (var lab = new ConversionLab(element))
                 {
-                    element.Remove();
+                    this.XRoot.SetAttributeValue(XNamespace.Xmlns + newNamespaceName, newNamespace.NamespaceName);
+                    lab.InsertUniqueElementBeforeTargetElement(new XElement(newNamespace + newElementName));
+
+                    if (replace)
+                    {
+                        lab.RemoveTargetElement();
+                    }
                 }
             }
         }
@@ -2241,7 +2242,7 @@ namespace WixToolset.Converters
                     element.Name = ns.GetName(element.Name.LocalName);
                 }
 
-                // Remove all the attributes and add them back to with their namespace updated (as necessary).
+                // Remove all the attributes and add them back with their namespace updated (as necessary).
                 IEnumerable<XAttribute> attributes = element.Attributes().ToList();
                 element.RemoveAttributes();
 
diff --git a/src/wix/test/WixToolsetTest.Converters/ConditionFixture.cs b/src/wix/test/WixToolsetTest.Converters/ConditionFixture.cs
index 24dee1c6..84e1b4c3 100644
--- a/src/wix/test/WixToolsetTest.Converters/ConditionFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/ConditionFixture.cs
@@ -187,6 +187,7 @@ namespace WixToolsetTest.Converters
             var actualLines = UnformattedDocumentLines(document);
             WixAssert.CompareLineByLine(expected, actualLines);
         }
+
         [Fact]
         public void FixPublishConditionWithComment()
         {
diff --git a/src/wix/test/WixToolsetTest.Converters/PrereqPackageFixture.cs b/src/wix/test/WixToolsetTest.Converters/PrereqPackageFixture.cs
index 1f095e67..5d637f65 100644
--- a/src/wix/test/WixToolsetTest.Converters/PrereqPackageFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/PrereqPackageFixture.cs
@@ -29,7 +29,8 @@ namespace WixToolsetTest.Converters
             var expected = new[]
             {
                 "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\" xmlns:bal=\"http://wixtoolset.org/schemas/v4/wxs/bal\">",
-                "  <Fragment><PackageGroup Id=\"NetFx452Web\">",
+                "  <Fragment>",
+                "    <PackageGroup Id=\"NetFx452Web\">",
                 "      <ExePackage Id=\"NetFx452Web\" bal:PrereqPackage=\"yes\" bal:PrereqLicenseUrl=\"$(var.NetFx452EulaLink)\" />",
                 "    </PackageGroup>",
                 "  </Fragment>",
diff --git a/src/wix/test/WixToolsetTest.Converters/VSExtensionFixture.cs b/src/wix/test/WixToolsetTest.Converters/VSExtensionFixture.cs
index 8f60cdf8..0cf8ae4b 100644
--- a/src/wix/test/WixToolsetTest.Converters/VSExtensionFixture.cs
+++ b/src/wix/test/WixToolsetTest.Converters/VSExtensionFixture.cs
@@ -28,7 +28,7 @@ namespace WixToolsetTest.Converters
                 "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\" xmlns:vs=\"http://wixtoolset.org/schemas/v4/wxs/vs\">",
                 "  <Fragment>",
                 "    <vs:FindVisualStudio />",
-                "    <vs:FindVisualStudio /><PropertyRef Id=\"VS2022_BOOTSTRAPPER_PACKAGE_FOLDER\" />",
+                "    <PropertyRef Id=\"VS2022_BOOTSTRAPPER_PACKAGE_FOLDER\" />",
                 "    <CustomActionRef Id=\"VS2017Setup\" />",
                 "  </Fragment>",
                 "</Wix>"