@joebigelow / wix-1 / commits / 40bd6537

Support converting deprecated inner text with embedded comments

Fixes 6847

Ron Martin committed Sep 10, 2022 at 15:44 UTC 40bd65379768f99ec28bffe2691ba43c78c9e9c4
4 files changed +581 -75
src/wix/WixToolset.Converters/ConversionLab.cs new
+91
@@ -0,0 +1,91 @@
1 +// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
2 +
3 +namespace WixToolset.Converters
4 +{
5 + using System;
6 + using System.Collections.Generic;
7 + using System.Linq;
8 + using System.Xml.Linq;
9 +
10 + internal class ConversionLab : IDisposable
11 + {
12 + private readonly XElement targetElement;
13 + private readonly XElement parentElement;
14 + private readonly List<XNode> siblingNodes;
15 + private int index;
16 +
17 + public ConversionLab(XElement targetElement)
18 + {
19 + this.targetElement = targetElement;
20 + this.parentElement = this.targetElement.Parent;
21 + this.siblingNodes = this.parentElement.Nodes().ToList();
22 +
23 + foreach (var siblingNode in this.siblingNodes)
24 + {
25 + siblingNode.Remove();
26 + }
27 +
28 + this.index = this.siblingNodes.IndexOf(this.targetElement);
29 + }
30 +
31 + public void RemoveOrphanTextNodes()
32 + {
33 + if (!this.targetElement.HasElements)
34 + {
35 + var childNodes = this.targetElement.Nodes().ToList();
36 + foreach (var childNode in childNodes)
37 + {
38 + childNode.Remove();
39 + }
40 + }
41 + }
42 +
43 + public void RemoveTargetElement()
44 + {
45 + if (this.index + 1 < this.siblingNodes.Count
46 + && this.siblingNodes[this.index + 1] is XText trailingText
47 + && String.IsNullOrWhiteSpace(trailingText.Value))
48 + {
49 + this.siblingNodes.RemoveAt(this.index + 1);
50 + }
51 + this.siblingNodes.RemoveAt(this.index);
52 + if (0 < this.index
53 + && this.siblingNodes[this.index - 1] is XText leadingText
54 + && String.IsNullOrWhiteSpace(leadingText.Value))
55 + {
56 + this.siblingNodes.RemoveAt(this.index - 1);
57 + }
58 + }
59 +
60 + public void ReplaceTargetElement(XElement replacement)
61 + {
62 + this.siblingNodes[this.index] = replacement;
63 + }
64 +
65 + public void AddCommentsAsSiblings(List<XNode> comments)
66 + {
67 + if (0 < this.index
68 + && this.siblingNodes[this.index - 1] is XText leadingText
69 + && String.IsNullOrWhiteSpace(leadingText.Value))
70 + {
71 + var leadingWhitespace = leadingText.Value;
72 + --this.index;
73 + var newComments = new List<XNode>();
74 + foreach(var comment in comments)
75 + {
76 + newComments.Add(new XText(leadingWhitespace));
77 + newComments.Add(comment);
78 + }
79 + comments = newComments;
80 + }
81 +
82 + this.siblingNodes.InsertRange(this.index, comments);
83 + this.index = this.siblingNodes.IndexOf(this.targetElement);
84 + }
85 +
86 + public void Dispose()
87 + {
88 + this.parentElement.Add(this.siblingNodes);
89 + }
90 + }
91 +}
src/wix/WixToolset.Converters/WixConverter.cs
+131 -59
@@ -525,7 +525,7 @@ namespace WixToolset.Converters
525 {
526 if (!WixConverter.LeadingWhitespaceValid(this.IndentationAmount, level, whitespace.Value))
527 {
528 - var message = testType == ConverterTestType.WhitespacePrecedingEndElementWrong ? "The whitespace preceding this end element is incorrect." : "The whitespace preceding this node is incorrect.";
528 + var message = testType == ConverterTestType.WhitespacePrecedingEndElementWrong ? "The whitespace preceding this end element is incorrect." : "The whitespace preceding this comment is incorrect.";
529
530 if (this.OnInformation(testType, node, message))
531 {
@@ -538,7 +538,7 @@ namespace WixToolset.Converters
538 {
539 if (!String.IsNullOrEmpty(whitespace.Value) && whitespace.NodeType != XmlNodeType.CDATA)
540 {
541 - var message = testType == ConverterTestType.WhitespacePrecedingEndElementWrong ? "The whitespace preceding this end element is incorrect." : "The whitespace preceding this node is incorrect.";
541 + var message = testType == ConverterTestType.WhitespacePrecedingEndElementWrong ? "The whitespace preceding this end element is incorrect." : "The whitespace preceding this comment is incorrect.";
542
543 if (this.OnInformation(testType, node, message))
544 {
@@ -924,23 +924,29 @@ namespace WixToolset.Converters
924
925 private void ConvertControlElement(XElement element)
926 {
927 - var remove = new List<XElement>();
928 -
929 - foreach (var xCondition in element.Elements(ConditionElementName))
927 + using (var lab = new ConversionLab(element))
928 {
931 - var action = UppercaseFirstChar(xCondition.Attribute("Action")?.Value);
932 - if (!String.IsNullOrEmpty(action) &&
933 - TryGetInnerText(xCondition, out var text) &&
934 - this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the '{1}Condition' attribute instead.", xCondition.Name.LocalName, action))
929 + var xConditions = element.Elements(ConditionElementName).ToList();
930 + var comments = new List<XNode>();
931 +
932 + foreach (var xCondition in xConditions)
933 {
936 - element.Add(new XAttribute(action + "Condition", text));
937 - remove.Add(xCondition);
934 + var action = UppercaseFirstChar(xCondition.Attribute("Action")?.Value);
935 + if (!String.IsNullOrEmpty(action) &&
936 + TryGetInnerText(xCondition, out var text, out comments, comments) &&
937 + this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the '{1}Condition' attribute instead.", xCondition.Name.LocalName, action))
938 + {
939 + element.Add(new XAttribute(action + "Condition", text));
940 + }
941 }
939 - }
942
941 - for (var i = remove.Count - 1; i >= 0; i--)
942 - {
943 - remove[i].Remove();
943 + foreach (var xCondition in xConditions)
944 + {
945 + xCondition.Remove();
946 + }
947 +
948 + lab.RemoveOrphanTextNodes();
949 + lab.AddCommentsAsSiblings(comments);
950 }
951 }
952
@@ -958,11 +964,16 @@ namespace WixToolset.Converters
964 var xCondition = element.Element(ConditionElementName);
965 if (xCondition != null)
966 {
961 - if (TryGetInnerText(xCondition, out var text) &&
967 + if (TryGetInnerText(xCondition, out var text, out var comments) &&
968 this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the 'Condition' attribute instead.", xCondition.Name.LocalName))
969 {
964 - element.Add(new XAttribute("Condition", text));
965 - xCondition.Remove();
970 + using (var lab = new ConversionLab(element))
971 + {
972 + xCondition.Remove();
973 + element.Add(new XAttribute("Condition", text));
974 + lab.RemoveOrphanTextNodes();
975 + lab.AddCommentsAsSiblings(comments);
976 + }
977 }
978 }
979
@@ -1057,14 +1068,16 @@ namespace WixToolset.Converters
1068 {
1069 var level = xCondition.Attribute("Level")?.Value;
1070 if (!String.IsNullOrEmpty(level) &&
1060 - TryGetInnerText(xCondition, out var text) &&
1071 + TryGetInnerText(xCondition, out var text, out var comments) &&
1072 this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the 'Level' element instead.", xCondition.Name.LocalName))
1073 {
1063 - xCondition.AddAfterSelf(new XElement(LevelElementName,
1064 - new XAttribute("Value", level),
1065 - new XAttribute("Condition", text)
1066 - ));
1067 - xCondition.Remove();
1074 + using (var lab = new ConversionLab(xCondition))
1075 + {
1076 + lab.ReplaceTargetElement(new XElement(LevelElementName,
1077 + new XAttribute("Value", level),
1078 + new XAttribute("Condition", text)));
1079 + lab.AddCommentsAsSiblings(comments);
1080 + }
1081 }
1082 }
1083 }
@@ -1097,28 +1110,25 @@ namespace WixToolset.Converters
1110
1111 private void ConvertFragmentElement(XElement element)
1112 {
1100 - var remove = new List<XElement>();
1113 + var xConditions = element.Elements(ConditionElementName).ToList();
1114
1102 - foreach (var xCondition in element.Elements(ConditionElementName))
1115 + foreach (var xCondition in xConditions)
1116 {
1117 var message = xCondition.Attribute("Message")?.Value;
1118
1119 if (!String.IsNullOrEmpty(message) &&
1107 - TryGetInnerText(xCondition, out var text) &&
1120 + TryGetInnerText(xCondition, out var text, out var comments) &&
1121 this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the 'Launch' element instead.", xCondition.Name.LocalName))
1122 {
1110 - xCondition.AddAfterSelf(new XElement(LaunchElementName,
1111 - new XAttribute("Condition", text),
1112 - new XAttribute("Message", message)
1113 - ));
1114 - remove.Add(xCondition);
1123 + using (var lab = new ConversionLab(xCondition))
1124 + {
1125 + lab.ReplaceTargetElement(new XElement(LaunchElementName,
1126 + new XAttribute("Condition", text),
1127 + new XAttribute("Message", message)));
1128 + lab.AddCommentsAsSiblings(comments);
1129 + }
1130 }
1131 }
1117 -
1118 - for (var i = remove.Count - 1; i >= 0; i--)
1119 - {
1120 - remove[i].Remove();
1121 - }
1132 }
1133
1134 private void ConvertFirewallRemoteAddressElement(XElement element)
@@ -1161,11 +1171,18 @@ namespace WixToolset.Converters
1171 var xCondition = element.Element(ConditionElementName);
1172 if (xCondition != null)
1173 {
1164 - if (TryGetInnerText(xCondition, out var text) &&
1174 + if (TryGetInnerText(xCondition, out var text, out var comments) &&
1175 this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the 'Condition' attribute instead.", xCondition.Name.LocalName))
1176 {
1167 - element.Add(new XAttribute("Condition", text));
1168 - xCondition.Remove();
1177 + using (var lab = new ConversionLab(xCondition))
1178 + {
1179 + lab.RemoveTargetElement();
1180 + }
1181 + using (var lab = new ConversionLab(element))
1182 + {
1183 + element.Add(new XAttribute("Condition", text));
1184 + lab.AddCommentsAsSiblings(comments);
1185 + }
1186 }
1187 }
1188 }
@@ -1251,14 +1268,16 @@ namespace WixToolset.Converters
1268 var message = xCondition.Attribute("Message")?.Value;
1269
1270 if (!String.IsNullOrEmpty(message) &&
1254 - TryGetInnerText(xCondition, out var text) &&
1271 + TryGetInnerText(xCondition, out var text, out var comments) &&
1272 this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the 'Launch' element instead.", xCondition.Name.LocalName))
1273 {
1257 - xCondition.AddAfterSelf(new XElement(LaunchElementName,
1258 - new XAttribute("Condition", text),
1259 - new XAttribute("Message", message)
1260 - ));
1261 - xCondition.Remove();
1274 + using (var lab = new ConversionLab(xCondition))
1275 + {
1276 + lab.ReplaceTargetElement(new XElement(LaunchElementName,
1277 + new XAttribute("Condition", text),
1278 + new XAttribute("Message", message)));
1279 + lab.AddCommentsAsSiblings(comments);
1280 + }
1281 }
1282 }
1283
@@ -1540,13 +1559,23 @@ namespace WixToolset.Converters
1559
1560 private void ConvertPublishElement(XElement element)
1561 {
1543 - this.ConvertInnerTextToAttribute(element, "Condition");
1544 -
1545 - var xCondition = element.Attribute("Condition");
1546 - if (xCondition?.Value == "1" &&
1547 - this.OnInformation(ConverterTestType.PublishConditionOneUnnecessary, element, "Adding Condition='1' on {0} elements is no longer necessary. Remove the Condition attribute.", xCondition.Name.LocalName))
1562 + if (TryGetInnerText(element, out var text, out var comments) &&
1563 + this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the 'Condition' attribute instead.", element.Name.LocalName))
1564 {
1549 - xCondition.Remove();
1565 + using (var lab = new ConversionLab(element))
1566 + {
1567 + if ("1" == text)
1568 + {
1569 + this.OnInformation(ConverterTestType.PublishConditionOneUnnecessary, element, "Adding Condition='1' on {0} elements is no longer necessary. Remove the Condition attribute.", element.Name.LocalName);
1570 + }
1571 + else
1572 + {
1573 + element.Add(new XAttribute("Condition", text));
1574 + }
1575 +
1576 + lab.RemoveOrphanTextNodes();
1577 + lab.AddCommentsAsSiblings(comments);
1578 + }
1579 }
1580 }
1581
@@ -1824,7 +1853,7 @@ namespace WixToolset.Converters
1853
1854 var xScript = xCustomAction.Attribute("Script");
1855
1827 - if (xScript != null && TryGetInnerText(xCustomAction, out var scriptText))
1856 + if (xScript != null && TryGetInnerText(xCustomAction, out var scriptText, out var comments))
1857 {
1858 if (this.OnInformation(ConverterTestType.InnerTextDeprecated, xCustomAction, "Using {0} element text is deprecated. Extract the text to a file and use the 'ScriptSourceFile' attribute to reference it.", xCustomAction.Name.LocalName))
1859 {
@@ -1837,6 +1866,25 @@ namespace WixToolset.Converters
1866
1867 RemoveChildren(xCustomAction);
1868 xCustomAction.Add(new XAttribute("ScriptSourceFile", scriptFile));
1869 +
1870 + if (comments.Any())
1871 + {
1872 + var remainingNodes = xCustomAction.NodesAfterSelf().ToList();
1873 + var replacementNodes = remainingNodes.Where(e => XmlNodeType.Text != e.NodeType);
1874 + foreach (var node in remainingNodes)
1875 + {
1876 + node.Remove();
1877 + }
1878 + foreach (var comment in comments)
1879 + {
1880 + xCustomAction.Add(comment);
1881 + xCustomAction.Add("\n");
1882 + }
1883 + foreach (var node in replacementNodes)
1884 + {
1885 + xCustomAction.Add(node);
1886 + }
1887 + }
1888 }
1889 }
1890 }
@@ -1956,11 +2004,15 @@ namespace WixToolset.Converters
2004
2005 private void ConvertInnerTextToAttribute(XElement element, string attributeName)
2006 {
1959 - if (TryGetInnerText(element, out var text) &&
2007 + if (TryGetInnerText(element, out var text, out var comments) &&
2008 this.OnInformation(ConverterTestType.InnerTextDeprecated, element, "Using {0} element text is deprecated. Use the '{1}' attribute instead.", element.Name.LocalName, attributeName))
2009 {
1962 - element.Add(new XAttribute(attributeName, text));
1963 - RemoveChildren(element);
2010 + using (var lab = new ConversionLab(element))
2011 + {
2012 + lab.RemoveOrphanTextNodes();
2013 + element.Add(new XAttribute(attributeName, text));
2014 + lab.AddCommentsAsSiblings(comments);
2015 + }
2016 }
2017 }
2018
@@ -2271,16 +2323,36 @@ namespace WixToolset.Converters
2323 return value;
2324 }
2325
2274 - private static bool TryGetInnerText(XElement element, out string value)
2326 + private static bool TryGetInnerText(XElement element, out string value, out List<XNode> comments)
2327 + {
2328 + return TryGetInnerText(element, out value, out comments, new List<XNode>());
2329 + }
2330 +
2331 + private static bool TryGetInnerText(XElement element, out string value, out List<XNode> comments, List<XNode> initialComments)
2332 {
2333 value = null;
2334 + comments = null;
2335 var found = false;
2336
2337 var nodes = element.Nodes().ToList();
2338 + comments = initialComments;
2339 + var nonCommentNodes = new List<XNode>();
2340 +
2341 + foreach(var node in nodes)
2342 + {
2343 + if (XmlNodeType.Comment == node.NodeType)
2344 + {
2345 + comments.Add(node);
2346 + }
2347 + else
2348 + {
2349 + nonCommentNodes.Add(node);
2350 + }
2351 + }
2352
2281 - if (nodes.Any() && nodes.All(e => e.NodeType == XmlNodeType.Text || e.NodeType == XmlNodeType.CDATA))
2353 + if (nonCommentNodes.Any() && nonCommentNodes.All(e => e.NodeType == XmlNodeType.Text || e.NodeType == XmlNodeType.CDATA))
2354 {
2283 - value = String.Join(String.Empty, nodes.Cast<XText>().Select(TrimTextValue));
2355 + value = String.Join(String.Empty, nonCommentNodes.Cast<XText>().Select(TrimTextValue));
2356 found = true;
2357 }
2358
src/wix/test/WixToolsetTest.Converters/ConditionFixture.cs
+291 -15
@@ -35,13 +35,55 @@ namespace WixToolsetTest.Converters
35 " <Fragment>",
36 " <UI>",
37 " <Dialog Id=\"Dlg1\">",
38 - " <Control Id=\"Control1\" DisableCondition=\"x=y\" HideCondition=\"a&lt;&gt;b\">",
39 - " ",
40 - " ",
38 + " <Control Id=\"Control1\" DisableCondition=\"x=y\" HideCondition=\"a&lt;&gt;b\" />",
39 + " </Dialog>",
40 + " </UI>",
41 + " </Fragment>",
42 + "</Wix>"
43 + };
44 +
45 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
46 +
47 + var messaging = new MockMessaging();
48 + var converter = new WixConverter(messaging, 2, null, null);
49 +
50 + var errors = converter.ConvertDocument(document);
51 + Assert.Equal(4, errors);
52 +
53 + var actualLines = UnformattedDocumentLines(document);
54 + WixAssert.CompareLineByLine(expected, actualLines);
55 + }
56 +
57 + [Fact]
58 + public void FixControlConditionWithComment()
59 + {
60 + var parse = String.Join(Environment.NewLine,
61 + "<?xml version=\"1.0\" encoding=\"utf-16\"?>",
62 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
63 + " <Fragment>",
64 + " <UI>",
65 + " <Dialog Id='Dlg1'>",
66 + " <Control Id='Control1'>",
67 + " <Condition Action='disable'><!-- Comment 1 -->x=y</Condition>",
68 + " <Condition Action='hide'><!-- Comment 2 -->a&lt;>b</Condition>",
69 " </Control>",
70 " </Dialog>",
71 " </UI>",
72 " </Fragment>",
73 + "</Wix>");
74 +
75 + var expected = new[]
76 + {
77 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
78 + " <Fragment>",
79 + " <UI>",
80 + " <Dialog Id=\"Dlg1\">",
81 + " <!-- Comment 1 -->",
82 + " <!-- Comment 2 -->",
83 + " <Control Id=\"Control1\" DisableCondition=\"x=y\" HideCondition=\"a&lt;&gt;b\" />",
84 + " </Dialog>",
85 + " </UI>",
86 + " </Fragment>",
87 "</Wix>"
88 };
89
@@ -102,6 +144,53 @@ namespace WixToolsetTest.Converters
144 var actualLines = UnformattedDocumentLines(document);
145 WixAssert.CompareLineByLine(expected, actualLines);
146 }
147 + [Fact]
148 + public void FixPublishConditionWithComment()
149 + {
150 + var parse = String.Join(Environment.NewLine,
151 + "<?xml version=\"1.0\" encoding=\"utf-16\"?>",
152 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
153 + " <Fragment>",
154 + " <UI>",
155 + " <Dialog Id='Dlg1'>",
156 + " <Control Id='Control1'>",
157 + " <Publish Value='abc'><!-- Comment 1 -->1&lt;2</Publish>",
158 + " <Publish Value='gone'><!-- Comment 2 -->1</Publish>",
159 + " </Control>",
160 + " </Dialog>",
161 + " </UI>",
162 + " </Fragment>",
163 + "</Wix>");
164 +
165 + var expected = new[]
166 + {
167 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
168 + " <Fragment>",
169 + " <UI>",
170 + " <Dialog Id=\"Dlg1\">",
171 + " <Control Id=\"Control1\">",
172 + " <!-- Comment 1 -->",
173 + " <Publish Value=\"abc\" Condition=\"1&lt;2\" />",
174 + " <!-- Comment 2 -->",
175 + " <Publish Value=\"gone\" />",
176 + " </Control>",
177 + " </Dialog>",
178 + " </UI>",
179 + " </Fragment>",
180 + "</Wix>"
181 + };
182 +
183 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
184 +
185 + var messaging = new MockMessaging();
186 + var converter = new WixConverter(messaging, 2, null, null);
187 +
188 + var errors = converter.ConvertDocument(document);
189 + Assert.Equal(5, errors);
190 +
191 + var actualLines = UnformattedDocumentLines(document);
192 + WixAssert.CompareLineByLine(expected, actualLines);
193 + }
194
195 [Fact]
196 public void FixComponentCondition()
@@ -120,10 +209,43 @@ namespace WixToolsetTest.Converters
209 {
210 "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
211 " <Fragment>",
123 - " <Component Id=\"Comp1\" Directory=\"ApplicationFolder\" Condition=\"1&lt;2\">",
124 - " ",
212 + " <Component Id=\"Comp1\" Directory=\"ApplicationFolder\" Condition=\"1&lt;2\" />",
213 + " </Fragment>",
214 + "</Wix>"
215 + };
216 +
217 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
218 +
219 + var messaging = new MockMessaging();
220 + var converter = new WixConverter(messaging, 2, null, null);
221 +
222 + var errors = converter.ConvertDocument(document);
223 + Assert.Equal(3, errors);
224 +
225 + var actualLines = UnformattedDocumentLines(document);
226 + WixAssert.CompareLineByLine(expected, actualLines);
227 + }
228 +
229 + [Fact]
230 + public void FixComponentConditionWithComment()
231 + {
232 + var parse = String.Join(Environment.NewLine,
233 + "<?xml version=\"1.0\" encoding=\"utf-16\"?>",
234 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
235 + " <Fragment>",
236 + " <Component Id='Comp1' Directory='ApplicationFolder'>",
237 + " <Condition><!-- Comment -->1&lt;2</Condition>",
238 " </Component>",
239 " </Fragment>",
240 + "</Wix>");
241 +
242 + var expected = new[]
243 + {
244 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
245 + " <Fragment>",
246 + " <!-- Comment -->",
247 + " <Component Id=\"Comp1\" Directory=\"ApplicationFolder\" Condition=\"1&lt;2\" />",
248 + " </Fragment>",
249 "</Wix>"
250 };
251
@@ -175,27 +297,24 @@ namespace WixToolsetTest.Converters
297 Assert.Equal(4, errors);
298 }
299
178 - [Fact(Skip = "Test demonstrates failure")]
300 + [Fact]
301 public void FixConditionWithComment()
302 {
303 var parse = String.Join(Environment.NewLine,
304 "<?xml version=\"1.0\" encoding=\"utf-16\"?>",
305 "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
306 " <Fragment>",
185 - " <Condition Message='commented cdata'>",
186 - " <!-- commented condition -->",
187 - " <![CDATA[cdatacontent]]>",
188 - " </Condition>",
307 + " <Condition Message='commented cdata'><!-- commented conditional -->",
308 + "<![CDATA[cdatacontent]]></Condition>",
309 " </Fragment>",
310 "</Wix>");
311
192 - //TODO: expected value is not set in stone but must have replaced Condition element with Launch and should have kept the comment.
312 var expected = new[]
313 {
314 "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
315 " <Fragment>",
316 + " <!-- commented conditional -->",
317 " <Launch Condition=\"cdatacontent\" Message=\"commented cdata\" />",
198 - " <!-- commented condition -->",
318 " </Fragment>",
319 "</Wix>"
320 };
@@ -210,7 +329,7 @@ namespace WixToolsetTest.Converters
329 var actualLines = UnformattedDocumentLines(document);
330 WixAssert.CompareLineByLine(expected, actualLines);
331
213 - Assert.Equal(2, errors);
332 + Assert.Equal(3, errors);
333 }
334
335 [Fact]
@@ -284,6 +403,43 @@ namespace WixToolsetTest.Converters
403 WixAssert.CompareLineByLine(expected, actualLines);
404 }
405
406 + [Fact]
407 + public void FixFeatureConditionWithComment()
408 + {
409 + var parse = String.Join(Environment.NewLine,
410 + "<?xml version=\"1.0\" encoding=\"utf-16\"?>",
411 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
412 + " <Fragment>",
413 + " <Feature Id='Feature1'>",
414 + " <Condition Level='0'><!-- Comment -->PROP = 1</Condition>",
415 + " </Feature>",
416 + " </Fragment>",
417 + "</Wix>");
418 +
419 + var expected = new[]
420 + {
421 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
422 + " <Fragment>",
423 + " <Feature Id=\"Feature1\">",
424 + " <!-- Comment -->",
425 + " <Level Value=\"0\" Condition=\"PROP = 1\" />",
426 + " </Feature>",
427 + " </Fragment>",
428 + "</Wix>"
429 + };
430 +
431 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
432 +
433 + var messaging = new MockMessaging();
434 + var converter = new WixConverter(messaging, 2, null, null);
435 +
436 + var errors = converter.ConvertDocument(document);
437 + Assert.Equal(3, errors);
438 +
439 + var actualLines = UnformattedDocumentLines(document);
440 + WixAssert.CompareLineByLine(expected, actualLines);
441 + }
442 +
443 [Fact]
444 public void FixLaunchCondition()
445 {
@@ -322,6 +478,46 @@ namespace WixToolsetTest.Converters
478 WixAssert.CompareLineByLine(expected, actualLines);
479 }
480
481 + [Fact]
482 + public void FixLaunchConditionWithComment()
483 + {
484 + var parse = String.Join(Environment.NewLine,
485 + "<?xml version=\"1.0\" encoding=\"utf-16\"?>",
486 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
487 + " <Fragment>",
488 + " <Condition Message='Stop the install'>",
489 + " <!-- Comment 1 -->1&lt;2",
490 + " </Condition>",
491 + " <Condition Message='Do not stop'>",
492 + " <!-- Comment 2 -->1=2",
493 + " </Condition>",
494 + " </Fragment>",
495 + "</Wix>");
496 +
497 + var expected = new[]
498 + {
499 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
500 + " <Fragment>",
501 + " <!-- Comment 1 -->",
502 + " <Launch Condition=\"1&lt;2\" Message=\"Stop the install\" />",
503 + " <!-- Comment 2 -->",
504 + " <Launch Condition=\"1=2\" Message=\"Do not stop\" />",
505 + " </Fragment>",
506 + "</Wix>"
507 + };
508 +
509 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
510 +
511 + var messaging = new MockMessaging();
512 + var converter = new WixConverter(messaging, 2, null, null);
513 +
514 + var errors = converter.ConvertDocument(document);
515 + Assert.Equal(4, errors);
516 +
517 + var actualLines = UnformattedDocumentLines(document);
518 + WixAssert.CompareLineByLine(expected, actualLines);
519 + }
520 +
521 [Fact]
522 public void FixLaunchConditionInProduct()
523 {
@@ -362,6 +558,48 @@ namespace WixToolsetTest.Converters
558 WixAssert.CompareLineByLine(expected, actualLines);
559 }
560
561 + [Fact]
562 + public void FixLaunchConditionInProductWithComment()
563 + {
564 + var parse = String.Join(Environment.NewLine,
565 + "<?xml version=\"1.0\" encoding=\"utf-16\"?>",
566 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
567 + " <Product>",
568 + " <Package />",
569 + " <Condition Message='Stop the install'>",
570 + " <!-- Comment 1 -->1&lt;2",
571 + " </Condition>",
572 + " <Condition Message='Do not stop'>",
573 + " <!-- Comment 2 -->1=2",
574 + " </Condition>",
575 + " </Product>",
576 + "</Wix>");
577 +
578 + var expected = new[]
579 + {
580 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
581 + " <Package Compressed=\"no\">",
582 + " ",
583 + " <!-- Comment 1 -->",
584 + " <Launch Condition=\"1&lt;2\" Message=\"Stop the install\" />",
585 + " <!-- Comment 2 -->",
586 + " <Launch Condition=\"1=2\" Message=\"Do not stop\" />",
587 + " </Package>",
588 + "</Wix>"
589 + };
590 +
591 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
592 +
593 + var messaging = new MockMessaging();
594 + var converter = new WixConverter(messaging, 2, null, null);
595 +
596 + var errors = converter.ConvertDocument(document);
597 + Assert.Equal(6, errors);
598 +
599 + var actualLines = UnformattedDocumentLines(document);
600 + WixAssert.CompareLineByLine(expected, actualLines);
601 + }
602 +
603 [Fact]
604 public void FixPermissionExCondition()
605 {
@@ -383,11 +621,49 @@ namespace WixToolsetTest.Converters
621 "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
622 " <Fragment>",
623 " <Component Id=\"Comp1\" Directory=\"ApplicationFolder\">",
386 - " <PermissionEx Sddl=\"sddl\" Condition=\"1&lt;2\">",
387 - " ",
624 + " <PermissionEx Sddl=\"sddl\" Condition=\"1&lt;2\" />",
625 + " </Component>",
626 + " </Fragment>",
627 + "</Wix>"
628 + };
629 +
630 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
631 +
632 + var messaging = new MockMessaging();
633 + var converter = new WixConverter(messaging, 2, null, null);
634 +
635 + var errors = converter.ConvertDocument(document);
636 + Assert.Equal(3, errors);
637 +
638 + var actualLines = UnformattedDocumentLines(document);
639 + WixAssert.CompareLineByLine(expected, actualLines);
640 + }
641 +
642 + [Fact]
643 + public void FixPermissionExConditionWithComment()
644 + {
645 + var parse = String.Join(Environment.NewLine,
646 + "<!-- Comment 1 -->",
647 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi'>",
648 + " <Fragment>",
649 + " <Component Id='Comp1' Guid='*' Directory='ApplicationFolder'>",
650 + " <PermissionEx Sddl='sddl'>",
651 + " <Condition><!-- Comment 2 -->1&lt;2</Condition>",
652 " </PermissionEx>",
653 " </Component>",
654 " </Fragment>",
655 + "</Wix>");
656 +
657 + var expected = new[]
658 + {
659 + "<!-- Comment 1 -->",
660 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\">",
661 + " <Fragment>",
662 + " <Component Id=\"Comp1\" Directory=\"ApplicationFolder\">",
663 + " <!-- Comment 2 -->",
664 + " <PermissionEx Sddl=\"sddl\" Condition=\"1&lt;2\" />",
665 + " </Component>",
666 + " </Fragment>",
667 "</Wix>"
668 };
669
src/wix/test/WixToolsetTest.Converters/UtilExtensionFixture.cs
+68 -1
@@ -44,6 +44,40 @@ namespace WixToolsetTest.Converters
44 WixAssert.CompareLineByLine(expected, actualLines);
45 }
46
47 + [Fact]
48 + public void FixCloseAppsConditionWithComment()
49 + {
50 + var parse = String.Join(Environment.NewLine,
51 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi' xmlns:util='http://schemas.microsoft.com/wix/UtilExtension'>",
52 + " <Fragment>",
53 + " <util:CloseApplication Id='EndApp' Target='example.exe'>",
54 + " <!-- Comment -->a&lt;>b",
55 + " </util:CloseApplication>",
56 + " </Fragment>",
57 + "</Wix>");
58 +
59 + var expected = new[]
60 + {
61 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\" xmlns:util=\"http://wixtoolset.org/schemas/v4/wxs/util\">",
62 + " <Fragment>",
63 + " <!-- Comment -->",
64 + " <util:CloseApplication Id=\"EndApp\" Target=\"example.exe\" Condition=\"a&lt;&gt;b\" />",
65 + " </Fragment>",
66 + "</Wix>"
67 + };
68 +
69 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
70 +
71 + var messaging = new MockMessaging();
72 + var converter = new WixConverter(messaging, 2, null, null);
73 +
74 + var errors = converter.ConvertDocument(document);
75 + Assert.Equal(3, errors);
76 +
77 + var actualLines = UnformattedDocumentLines(document);
78 + WixAssert.CompareLineByLine(expected, actualLines);
79 + }
80 +
81 [Fact]
82 public void FixXmlConfigValue()
83 {
@@ -77,6 +111,40 @@ namespace WixToolsetTest.Converters
111 WixAssert.CompareLineByLine(expected, actualLines);
112 }
113
114 + [Fact]
115 + public void FixXmlConfigValueWithComment()
116 + {
117 + var parse = String.Join(Environment.NewLine,
118 + "<Wix xmlns='http://schemas.microsoft.com/wix/2006/wi' xmlns:util='http://schemas.microsoft.com/wix/UtilExtension'>",
119 + " <Fragment>",
120 + " <util:XmlConfig Id='Change' ElementPath='book'>",
121 + " <!-- Comment -->a&lt;>b",
122 + " </util:XmlConfig>",
123 + " </Fragment>",
124 + "</Wix>");
125 +
126 + var expected = new[]
127 + {
128 + "<Wix xmlns=\"http://wixtoolset.org/schemas/v4/wxs\" xmlns:util=\"http://wixtoolset.org/schemas/v4/wxs/util\">",
129 + " <Fragment>",
130 + " <!-- Comment -->",
131 + " <util:XmlConfig Id=\"Change\" ElementPath=\"book\" Value=\"a&lt;&gt;b\" />",
132 + " </Fragment>",
133 + "</Wix>"
134 + };
135 +
136 + var document = XDocument.Parse(parse, LoadOptions.PreserveWhitespace | LoadOptions.SetLineInfo);
137 +
138 + var messaging = new MockMessaging();
139 + var converter = new WixConverter(messaging, 2, null, null);
140 +
141 + var errors = converter.ConvertDocument(document);
142 + Assert.Equal(3, errors);
143 +
144 + var actualLines = UnformattedDocumentLines(document);
145 + WixAssert.CompareLineByLine(expected, actualLines);
146 + }
147 +
148 [Fact]
149 public void WarnsOnAllRegistryValueSearches()
150 {
@@ -112,7 +180,6 @@ namespace WixToolsetTest.Converters
180 WixAssert.CompareLineByLine(expected, actualLines);
181 }
182
115 -
183 [Fact]
184 public void FixXmlConfigValueCData()
185 {