Merge pull request #657 from gumme/WpfDesignerCollectionResetUndoRedoFix.

src/AddIns/DisplayBindings/WpfDesign/WpfDesign.Designer/Project/Xaml/XamlModelCollectionElementsCollection.cs

@@ -164,6 +164,11 @@ namespace ICSharpCode.WpfDesign.Designer.Xaml
 			Execute(new RemoveAtAction(this, index, (XamlDesignItem)this[index]));
 		}
 		
+		internal ITransactionItem CreateResetTransaction()
+		{
+			return new ResetAction(this);
+		}
+		
 		void Execute(ITransactionItem item)
 		{
 			UndoService undoService = context.Services.GetService<UndoService>();
@@ -279,5 +284,60 @@ namespace ICSharpCode.WpfDesign.Designer.Xaml
 				return false;
 			}
 		}
+		
+		sealed class ResetAction : ITransactionItem
+		{
+			readonly XamlModelCollectionElementsCollection collection;
+			readonly XamlDesignItem[] items;
+			
+			public ResetAction(XamlModelCollectionElementsCollection collection)
+			{
+				this.collection = collection;
+				
+				items = new XamlDesignItem[collection.Count];
+				for (int i = 0; i < collection.Count; i++) {
+					items[i] = (XamlDesignItem)collection[i];
+				}
+			}
+			
+			#region ITransactionItem implementation
+			
+			public void Do()
+			{
+				for (int i = items.Length - 1; i >= 0; i--) {
+					collection.RemoveInternal(i, items[i]);
+				}
+				collection.modelProperty.XamlDesignItem.NotifyPropertyChanged(collection.modelProperty);
+			}
+			public void Undo()
+			{
+				for (int i = 0; i < items.Length; i++) {
+					collection.InsertInternal(i, items[i]);
+				}
+				collection.modelProperty.XamlDesignItem.NotifyPropertyChanged(collection.modelProperty);
+			}
+			public bool MergeWith(ITransactionItem other)
+			{
+				return false;
+			}
+			
+			#endregion
+			
+			#region IUndoAction implementation
+			
+			public ICollection<DesignItem> AffectedElements {
+				get {
+					return new DesignItem[] { collection.modelProperty.DesignItem };
+				}
+			}
+			
+			public string Title {
+				get {
+					return "Reset collection";
+				}
+			}
+			
+			#endregion
+		}
 	}
 }

src/AddIns/DisplayBindings/WpfDesign/WpfDesign.Designer/Project/Xaml/XamlModelProperty.cs

@@ -249,11 +249,12 @@ namespace ICSharpCode.WpfDesign.Designer.Xaml
 		
 		public sealed class PropertyChangeAction : ITransactionItem
 		{
-			XamlModelProperty property;
-			XamlPropertyValue oldValue;
+			readonly XamlModelProperty property;
+			readonly XamlPropertyValue oldValue;
 			XamlPropertyValue newValue;
-			bool oldIsSet;
+			readonly bool oldIsSet;
 			bool newIsSet;
+			readonly ITransactionItem collectionTransactionItem;
 			
 			public PropertyChangeAction(XamlModelProperty property, XamlPropertyValue newValue, bool newIsSet)
 			{
@@ -263,6 +264,10 @@ namespace ICSharpCode.WpfDesign.Designer.Xaml
 				
 				oldIsSet = property._property.IsSet;
 				oldValue = property._property.PropertyValue;
+				
+				if (oldIsSet && oldValue == null && property.IsCollection) {
+					collectionTransactionItem = property._collectionElements.CreateResetTransaction();
+				}
 			}
 			
 			public string Title {
@@ -276,6 +281,10 @@ namespace ICSharpCode.WpfDesign.Designer.Xaml
 			
 			public void Do()
 			{
+				if (collectionTransactionItem != null) {
+					collectionTransactionItem.Do();
+				}
+				
 				if (newIsSet)
 					property.SetValueInternal(newValue);
 				else
@@ -284,8 +293,13 @@ namespace ICSharpCode.WpfDesign.Designer.Xaml
 			
 			public void Undo()
 			{
-				if (oldIsSet)
-					property.SetValueInternal(oldValue);
+				if (oldIsSet) {
+					if (collectionTransactionItem != null) {
+						collectionTransactionItem.Undo();
+					} else {
+						property.SetValueInternal(oldValue);
+					}
+				}
 				else
 					property.ResetInternal();
 			}

src/AddIns/DisplayBindings/WpfDesign/WpfDesign.Designer/Tests/Designer/ModelTests.cs

@@ -19,6 +19,7 @@
 using System;
 using System.Diagnostics;
 using System.IO;
+using System.Linq;
 using System.Text;
 using System.Windows;
 using System.Windows.Controls;
@@ -50,7 +51,7 @@ namespace ICSharpCode.WpfDesign.Tests.Designer
 		{
 			DesignItem button = CreateCanvasContext("<Button><Button.Width>50</Button.Width></Button>");
 			button.Properties["Width"].SetValue(100.0);
-			AssertCanvasDesignerOutput("<Button Width=\"100\">\n</Button>", button.Context);
+			AssertCanvasDesignerOutput("<Button Width=\"100\" />", button.Context);
 			AssertLog("");
 		}
 		
@@ -163,7 +164,8 @@ namespace ICSharpCode.WpfDesign.Tests.Designer
 
 		void UndoRedoListInternal(bool useExplicitList)
 		{
-			DesignItem button = CreateCanvasContext("<Button/>");
+			const string originalXaml = "<Button />";
+			DesignItem button = CreateCanvasContext(originalXaml);
 			UndoService s = button.Context.Services.GetService<UndoService>();
 			IComponentService component = button.Context.Services.Component;
 			string expectedXamlWithList;
@@ -221,7 +223,7 @@ namespace ICSharpCode.WpfDesign.Tests.Designer
 			s.Undo();
 			Assert.IsFalse(s.CanUndo);
 			Assert.IsTrue(s.CanRedo);
-			AssertCanvasDesignerOutput("<Button>\n</Button>", button.Context, "xmlns:Controls0=\"" + ICSharpCode.WpfDesign.Tests.XamlDom.XamlTypeFinderTests.XamlDomTestsNamespace + "\"");
+			AssertCanvasDesignerOutput(originalXaml, button.Context, "xmlns:Controls0=\"" + ICSharpCode.WpfDesign.Tests.XamlDom.XamlTypeFinderTests.XamlDomTestsNamespace + "\"");
 			
 			s.Redo();
 			Assert.IsTrue(s.CanUndo);
@@ -248,7 +250,8 @@ namespace ICSharpCode.WpfDesign.Tests.Designer
 
 		void UndoRedoDictionaryInternal(bool useExplicitDictionary)
 		{
-			DesignItem button = CreateCanvasContext("<Button/>");
+			const string originalXaml = "<Button />";
+			DesignItem button = CreateCanvasContext(originalXaml);
 			UndoService s = button.Context.Services.GetService<UndoService>();
 			IComponentService component = button.Context.Services.Component;
 			string expectedXamlWithDictionary;
@@ -307,7 +310,7 @@ namespace ICSharpCode.WpfDesign.Tests.Designer
 			s.Undo();
 			Assert.IsFalse(s.CanUndo);
 			Assert.IsTrue(s.CanRedo);
-			AssertCanvasDesignerOutput("<Button>\n</Button>", button.Context, "xmlns:Controls0=\"" + ICSharpCode.WpfDesign.Tests.XamlDom.XamlTypeFinderTests.XamlDomTestsNamespace + "\"");
+			AssertCanvasDesignerOutput(originalXaml, button.Context, "xmlns:Controls0=\"" + ICSharpCode.WpfDesign.Tests.XamlDom.XamlTypeFinderTests.XamlDomTestsNamespace + "\"");
 			
 			s.Redo();
 			Assert.IsTrue(s.CanUndo);
@@ -320,6 +323,179 @@ namespace ICSharpCode.WpfDesign.Tests.Designer
 			AssertLog("");
 		}
 		
+		[Test]
+		public void UndoRedoInputBindings()
+		{
+			const string originalXaml =  "<TextBlock Text=\"My text\" />";
+			
+			DesignItem textBlock = CreateCanvasContext(originalXaml);
+			UndoService s = textBlock.Context.Services.GetService<UndoService>();
+			IComponentService component = textBlock.Context.Services.Component;
+			
+			Assert.IsFalse(s.CanUndo);
+			Assert.IsFalse(s.CanRedo);
+			
+			DesignItemProperty inputbinding = textBlock.Properties["InputBindings"];
+			Assert.IsTrue(inputbinding.IsCollection);
+			
+			const string expectedXaml = @"<TextBlock Text=""My text"">
+  <TextBlock.InputBindings>
+    <MouseBinding Gesture=""LeftDoubleClick"" Command=""ApplicationCommands.New"" />
+  </TextBlock.InputBindings>
+</TextBlock>";
+			
+			using (ChangeGroup changeGroup = textBlock.Context.OpenGroup("", new[] { textBlock }))
+			{
+				DesignItem di = component.RegisterComponentForDesigner(new System.Windows.Input.MouseBinding());
+				di.Properties["Gesture"].SetValue(System.Windows.Input.MouseAction.LeftDoubleClick);
+				di.Properties["Command"].SetValue("ApplicationCommands.New");
+
+				inputbinding.CollectionElements.Add(di);
+
+				changeGroup.Commit();
+			}
+			
+			Assert.IsTrue(s.CanUndo);
+			Assert.IsFalse(s.CanRedo);
+			AssertCanvasDesignerOutput(expectedXaml, textBlock.Context);
+			
+			inputbinding = textBlock.Properties["InputBindings"];
+			Assert.IsTrue(((System.Windows.Input.InputBindingCollection)inputbinding.ValueOnInstance).Count == inputbinding.CollectionElements.Count);
+			
+			s.Undo();
+			Assert.IsFalse(s.CanUndo);
+			Assert.IsTrue(s.CanRedo);
+			AssertCanvasDesignerOutput(originalXaml, textBlock.Context);
+			
+			s.Redo();
+			Assert.IsTrue(s.CanUndo);
+			Assert.IsFalse(s.CanRedo);
+			AssertCanvasDesignerOutput(expectedXaml, textBlock.Context);
+			
+			Assert.IsTrue(((System.Windows.Input.InputBindingCollection)inputbinding.ValueOnInstance).Count == inputbinding.CollectionElements.Count);
+			
+			AssertLog("");
+		}
+		
+		void UndoRedoInputBindingsRemoveClearResetInternal(bool remove, bool clear, bool reset)
+		{
+			const string originalXaml =  "<TextBlock Text=\"My text\" />";
+			
+			DesignItem textBlock = CreateCanvasContext(originalXaml);
+			UndoService s = textBlock.Context.Services.GetService<UndoService>();
+			IComponentService component = textBlock.Context.Services.Component;
+			
+			Assert.IsFalse(s.CanUndo);
+			Assert.IsFalse(s.CanRedo);
+			
+			DesignItemProperty inputbinding = textBlock.Properties["InputBindings"];
+			Assert.IsTrue(inputbinding.IsCollection);
+			
+			const string expectedXaml = @"<TextBlock Text=""My text"" Cursor=""Hand"">
+  <TextBlock.InputBindings>
+    <MouseBinding Gesture=""LeftDoubleClick"" Command=""ApplicationCommands.New"" />
+  </TextBlock.InputBindings>
+</TextBlock>";
+			
+			using (ChangeGroup changeGroup = textBlock.Context.OpenGroup("", new[] { textBlock }))
+			{
+				DesignItem di = component.RegisterComponentForDesigner(new System.Windows.Input.MouseBinding());
+				di.Properties["Gesture"].SetValue(System.Windows.Input.MouseAction.LeftDoubleClick);
+				di.Properties["Command"].SetValue("ApplicationCommands.New");
+
+				inputbinding.CollectionElements.Add(di);
+				
+				textBlock.Properties["Cursor"].SetValue(System.Windows.Input.Cursors.Hand);
+
+				changeGroup.Commit();
+			}
+			
+			Assert.IsTrue(s.CanUndo);
+			Assert.IsFalse(s.CanRedo);
+			AssertCanvasDesignerOutput(expectedXaml, textBlock.Context);
+			
+			using (ChangeGroup changeGroup = textBlock.Context.OpenGroup("", new[] { textBlock }))
+			{
+				DesignItem di = inputbinding.CollectionElements.First();
+				
+				// Remove, Clear, Reset combination caused exception at first Undo after this group commit before the issue was fixed
+				if (remove)
+					inputbinding.CollectionElements.Remove(di);
+				if (clear)
+					inputbinding.CollectionElements.Clear();
+				if (reset)
+					inputbinding.Reset();
+				
+				di = component.RegisterComponentForDesigner(new System.Windows.Input.MouseBinding());
+				di.Properties["Gesture"].SetValue(System.Windows.Input.MouseAction.LeftDoubleClick);
+				di.Properties["Command"].SetValue("ApplicationCommands.New");
+
+				inputbinding.CollectionElements.Add(di);
+				
+				textBlock.Properties["Cursor"].SetValue(System.Windows.Input.Cursors.Hand);
+
+				changeGroup.Commit(); 
+			}
+			
+			s.Undo();
+			s.Undo();
+			Assert.IsFalse(s.CanUndo);
+			Assert.IsTrue(s.CanRedo);
+			AssertCanvasDesignerOutput(originalXaml, textBlock.Context);
+			
+			s.Redo();
+			s.Redo();
+			Assert.IsTrue(s.CanUndo);
+			Assert.IsFalse(s.CanRedo);
+			AssertCanvasDesignerOutput(expectedXaml, textBlock.Context);
+			
+			Assert.IsTrue(((System.Windows.Input.InputBindingCollection)inputbinding.ValueOnInstance).Count == inputbinding.CollectionElements.Count);
+			
+			AssertLog("");
+		}
+		
+		[Test]
+		public void UndoRedoInputBindingsRemoveClearReset()
+		{
+			UndoRedoInputBindingsRemoveClearResetInternal(true, true, true);
+		}
+		
+		[Test]
+		public void UndoRedoInputBindingsRemove()
+		{
+			UndoRedoInputBindingsRemoveClearResetInternal(true, false, false);
+		}
+		
+		[Test]
+		public void UndoRedoInputBindingsClear()
+		{
+			UndoRedoInputBindingsRemoveClearResetInternal(false, true, false);
+		}
+		
+		[Test]
+		public void UndoRedoInputBindingsReset()
+		{
+			UndoRedoInputBindingsRemoveClearResetInternal(false, false, true);
+		}
+		
+		[Test]
+		public void UndoRedoInputBindingsRemoveClear()
+		{
+			UndoRedoInputBindingsRemoveClearResetInternal(true, true, false);
+		}
+		
+		[Test]
+		public void UndoRedoInputBindingsRemoveReset()
+		{
+			UndoRedoInputBindingsRemoveClearResetInternal(true, false, true);
+		}
+		
+		[Test]
+		public void UndoRedoInputBindingsClearReset()
+		{
+			UndoRedoInputBindingsRemoveClearResetInternal(false, true, true);
+		}
+		
 		[Test]
 		public void AddTextBoxToCanvas()
 		{

src/AddIns/DisplayBindings/WpfDesign/WpfDesign.Designer/Tests/XamlDom/CollectionTests.cs

@@ -20,5 +20,55 @@ namespace ICSharpCode.WpfDesign.Tests.XamlDom
 
             Assert.IsFalse(isCollection);
         }
+        
+        /// <summary>
+        /// This test is here do demonstrate a peculiarity (or bug) with collections inside System.Windows.Input namespace.
+        /// The problem is that inserting the first item into the collection will silently fail (Count still 0 after Insert), first item must use Add method.
+        /// </summary>
+        [Test]
+		public void InputCollectionsPeculiarityOrBug()
+		{
+			// InputBindingCollection START
+			var inputBindingCollection = new System.Windows.Input.InputBindingCollection();
+			
+			// NOTE: this silently fails (Count is 0 after insert)
+			inputBindingCollection.Insert(0, new System.Windows.Input.MouseBinding());
+			Assert.IsTrue(inputBindingCollection.Count == 0);
+			
+			inputBindingCollection.Add(new System.Windows.Input.MouseBinding());
+			Assert.IsTrue(inputBindingCollection.Count == 1);
+			
+			inputBindingCollection.Insert(0, new System.Windows.Input.MouseBinding());
+			Assert.IsTrue(inputBindingCollection.Count == 2);
+			// InputBindingCollection END
+			
+			// CommandBindingCollection START
+			var commandBindingCollection = new System.Windows.Input.CommandBindingCollection();
+			
+			// NOTE: this silently fails (Count is 0 after insert)
+			commandBindingCollection.Insert(0, new System.Windows.Input.CommandBinding());
+			Assert.IsTrue(commandBindingCollection.Count == 0);
+			
+			commandBindingCollection.Add(new System.Windows.Input.CommandBinding());
+			Assert.IsTrue(commandBindingCollection.Count == 1);
+			
+			commandBindingCollection.Insert(0, new System.Windows.Input.CommandBinding());
+			Assert.IsTrue(commandBindingCollection.Count == 2);
+			// CommandBindingCollection END
+			
+			// List START (how it probably should work...)
+			var list = new List<string>();
+			
+			// NOTE: this is successful for ordinary List<T>
+			list.Insert(0, "A");
+			Assert.IsTrue(list.Count == 1);
+			
+			list.Add("A");
+			Assert.IsTrue(list.Count == 2);
+			
+			list.Insert(0, "A");
+			Assert.IsTrue(list.Count == 3);
+			// List END
+		}
     }
 }

src/AddIns/DisplayBindings/WpfDesign/WpfDesign.XamlDom/Project/CollectionElementsCollection.cs

@@ -19,15 +19,17 @@
 using System;
 using System.Collections.ObjectModel;
 using System.Collections.Generic;
+using System.Collections.Specialized;
 
 namespace ICSharpCode.WpfDesign.XamlDom
 {
 	/// <summary>
 	/// The collection used by XamlProperty.CollectionElements
 	/// </summary>
-	sealed class CollectionElementsCollection : Collection<XamlPropertyValue>
+	sealed class CollectionElementsCollection : Collection<XamlPropertyValue>, INotifyCollectionChanged
 	{
 		XamlProperty property;
+		bool isClearing = false;
 		
 		internal CollectionElementsCollection(XamlProperty property)
 		{
@@ -44,9 +46,17 @@ namespace ICSharpCode.WpfDesign.XamlDom
 		
 		protected override void ClearItems()
 		{
-			while (Count > 0) {
-				RemoveAt(Count - 1);
+			isClearing = true;
+			try {
+				while (Count > 0) {
+					RemoveAt(Count - 1);
+				}
+			} finally {
+				isClearing = false;
 			}
+			
+			if (CollectionChanged != null)
+				CollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
 		}
 		
 		protected override void RemoveItem(int index)
@@ -58,9 +68,13 @@ namespace ICSharpCode.WpfDesign.XamlDom
 				CollectionSupport.RemoveItem(info.ReturnType, collection, propertyValue.GetValueFor(info), propertyValue);
 			}
 			
-			this[index].RemoveNodeFromParent();
-			this[index].ParentProperty = null;
+			var item = this[index];
+			item.RemoveNodeFromParent();
+			item.ParentProperty = null;
 			base.RemoveItem(index);
+			
+			if (CollectionChanged != null && !isClearing)
+				CollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index));
 		}
 		
 		protected override void InsertItem(int index, XamlPropertyValue item)
@@ -75,12 +89,25 @@ namespace ICSharpCode.WpfDesign.XamlDom
 			property.InsertNodeInCollection(item.GetNodeForCollection(), index);
 			
 			base.InsertItem(index, item);
+			
+			if (CollectionChanged != null)
+				CollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index));
 		}
 		
 		protected override void SetItem(int index, XamlPropertyValue item)
 		{
+			var oldItem = this[index];
 			RemoveItem(index);
 			InsertItem(index, item);
+			
+			if (CollectionChanged != null)
+				CollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, item, oldItem, index));
 		}
+
+		#region INotifyCollectionChanged implementation
+
+		public event NotifyCollectionChangedEventHandler CollectionChanged;
+
+		#endregion
 	}
 }

src/AddIns/DisplayBindings/WpfDesign/WpfDesign.XamlDom/Project/CollectionSupport.cs

@@ -108,16 +108,32 @@ namespace ICSharpCode.WpfDesign.XamlDom
 		/// </summary>
 		public static bool Insert(Type collectionType, object collectionInstance, XamlPropertyValue newElement, int index)
 		{
-			var hasInsert = collectionType.GetMethods().Any(x => x.Name == "Insert");
+			object value = newElement.GetValueFor(null);
 			
-			if (hasInsert) {
-				collectionType.InvokeMember(
-					"Insert", BindingFlags.Public | BindingFlags.InvokeMethod | BindingFlags.Instance,
-					null, collectionInstance,
-					new object[] { index, newElement.GetValueFor(null) },
-					CultureInfo.InvariantCulture);
-				
+			// Using IList, with possible Add instead of Insert, was primarily added as a workaround
+			// for a peculiarity (or bug) with collections inside System.Windows.Input namespace.
+			// See CollectionTests.InputCollectionsPeculiarityOrBug test method for details.
+			var list = collectionInstance as IList;
+			if (list != null) {
+				if (list.Count == index) {
+					list.Add(value);
+				}
+				else {
+					list.Insert(index, value);
+				}
 				return true;
+			} else {
+				var hasInsert = collectionType.GetMethods().Any(x => x.Name == "Insert");
+			
+				if (hasInsert) {
+					collectionType.InvokeMember(
+						"Insert", BindingFlags.Public | BindingFlags.InvokeMethod | BindingFlags.Instance,
+						null, collectionInstance,
+						new object[] { index, value },
+						CultureInfo.InvariantCulture);
+				
+					return true;
+				}
 			}
 			
 			return false;

src/AddIns/DisplayBindings/WpfDesign/WpfDesign.XamlDom/Project/XamlObject.cs

@@ -267,6 +267,10 @@ namespace ICSharpCode.WpfDesign.XamlDom
 			}
 			UpdateMarkupExtensionChain();
 			
+			if (!element.HasChildNodes && !element.IsEmpty) {
+				element.IsEmpty = true;
+			}
+			
 			if (property == NameProperty) {
 				if (NameChanged != null)
 					NameChanged(this, EventArgs.Empty);

src/AddIns/DisplayBindings/WpfDesign/WpfDesign.XamlDom/Project/XamlProperty.cs

@@ -67,6 +67,7 @@ namespace ICSharpCode.WpfDesign.XamlDom
 			if (propertyInfo.IsCollection) {
 				isCollection = true;
 				collectionElements = new CollectionElementsCollection(this);
+				collectionElements.CollectionChanged += OnCollectionChanged;
 				
 				if (propertyInfo.Name.Equals(XamlConstants.ResourcesPropertyName, StringComparison.Ordinal) &&
 				    propertyInfo.ReturnType == typeof(ResourceDictionary)) {
@@ -75,6 +76,25 @@ namespace ICSharpCode.WpfDesign.XamlDom
 			}
 		}
 		
+		void OnCollectionChanged(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e)
+		{
+			// If implicit collection that is now empty we remove markup for the property if still there.
+			if (collectionElements.Count == 0 && propertyValue == null && _propertyElement != null)
+			{
+				_propertyElement.ParentNode.RemoveChild(_propertyElement);
+				_propertyElement = null;
+				
+				ParentObject.OnPropertyChanged(this);
+				
+				if (IsSetChanged != null) {
+					IsSetChanged(this, EventArgs.Empty);
+				}
+				if (ValueChanged != null) {
+					ValueChanged(this, EventArgs.Empty);
+				}
+			}
+		}
+		
 		/// <summary>
 		/// Gets the parent object for which this property was declared.
 		/// </summary>