Revert previous XPath fix. Fix is rather needed in XPathNodeIterator.GetEnumerator().

The thing is, XPathNodeIterator itself is stateful and does not behave
like IEnumerable, thus GetEnumerator() should not iterate nodes within
*itself*. Rather, it should have returned a clone of it.

After all we didn't have to damage performance to fix bug #3705, which is nice.

mcs/class/System.XML/Makefile

@@ -77,14 +77,14 @@ EXTRA_DISTFILES = \
 	$(nist_dom_files:%=Test/System.Xml/nist_dom/%)
 
 System.Xml.XPath/Parser.cs: System.Xml.XPath/Parser.jay $(topdir)/jay/skeleton.cs
-	$(topdir)/jay/jay -ct < $(topdir)/jay/skeleton.cs $(CURDIR)/$< >$@
+	$(topdir)/jay/jay -ct < $(topdir)/jay/skeleton.cs $< >$@
 
 Mono.Xml.Xsl/PatternParser.jay: System.Xml.XPath/Parser.jay $(topdir)/jay/skeleton.cs
 	sed "s/\%start Expr/\%start Pattern/" $< >$@
 
 Mono.Xml.Xsl/PatternParser.cs: Mono.Xml.Xsl/PatternParser.jay $(topdir)/jay/skeleton.cs
 	echo "#define XSLT_PATTERN" > $@
-	$(topdir)/jay/jay -ct $(CURDIR)/$< < $(topdir)/jay/skeleton.cs >>$@
+	$(topdir)/jay/jay -ct $< < $(topdir)/jay/skeleton.cs >>$@
 
 Mono.Xml.Xsl/PatternTokenizer.cs: System.Xml.XPath/Tokenizer.cs
 	echo "#define XSLT_PATTERN" > $@

mcs/class/System.XML/System.Xml.XPath/Iterator.cs

@@ -854,8 +854,11 @@ namespace System.Xml.XPath
 					return false;
 				_right = _expr.EvaluateNodeSet (_left);
 			}
-			_current = _right.Current.Clone ();
-
+			if (_current == null)
+				_current = _right.Current.Clone ();
+			else
+				if (! _current.MoveTo (_right.Current) )
+					_current = _right.Current.Clone ();
 			return true;
 		}
 
@@ -1030,7 +1033,6 @@ namespace System.Xml.XPath
 		private BaseIterator _iter;
 		private Expression _pred;
 		private XPathResultType resType;
-		private XPathNavigator _current;
 		private bool finished;
 
 		public PredicateIterator (BaseIterator iter, Expression pred) : base (iter.NamespaceManager)
@@ -1077,20 +1079,17 @@ namespace System.Xml.XPath
 						break;
 				}
 
-				_current = _iter.Current.Clone ();
-
 				return true;
 			}
 			finished = true;
 			return false;
 		}
 		public override XPathNavigator Current {
-			get { return _current; }
-		}
+			get { return CurrentPosition == 0 ? null : _iter.Current; }}
 		public override bool ReverseAxis {
 			get { return _iter.ReverseAxis; }
 		}
-		public override string ToString () { return _iter.GetType ().FullName; }
+public override string ToString () { return _iter.GetType ().FullName; }
 	}
 
 	internal class ListIterator : BaseIterator

mcs/class/System.XML/System.Xml.XPath/XPathNodeIterator.cs

@@ -88,13 +88,40 @@ namespace System.Xml.XPath
 #if NET_2_0
 		public virtual IEnumerator GetEnumerator ()
 		{
-			while (MoveNext ())
-				yield return Current;
+			return new XPathNodeIteratorEnumerator (this);
 		}
 #endif
 
 		public abstract bool MoveNext ();
 		
 		#endregion
+
+		struct XPathNodeIteratorEnumerator : IEnumerator
+		{
+			XPathNodeIterator source;
+			XPathNavigator current;
+			public XPathNodeIteratorEnumerator (XPathNodeIterator source)
+			{
+				this.source = source.Clone ();
+				current = null;
+			}
+
+			public bool MoveNext ()
+			{
+				if (!source.MoveNext ())
+					return false;
+				current = source.Current.Clone ();
+				return true;
+			}
+			
+			public object Current {
+				get { return current; }
+			}
+
+			public void Reset ()
+			{
+				throw new NotSupportedException ();
+			}
+		}
 	}
 }

mcs/class/System.XML/Test/System.Xml.XPath/SelectNodesTests.cs

@@ -11,6 +11,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Xml;
 using System.Xml.XPath;
 
@@ -463,6 +464,7 @@ namespace MonoTests.System.Xml.XPath
 			Assert.AreEqual (5, nodes.Count, "#2");
 		}
 
+		// This test requires Linq.
 		[Test] // xamarin bug #3705
 		public void ReturnedNavigatorInstancesUnique ()
 		{
@@ -477,19 +479,28 @@ namespace MonoTests.System.Xml.XPath
 			var data = new XPathDocument (r);
 			var l = new List<XPathNavigator> ();
 
-			// test PredicateIterator.
+			// test PredicateIterator wrapped by XPathNodeIteratorEnumerator.
 			var parent = data.CreateNavigator ().SelectSingleNode ("/Notes");
 			foreach (XPathNavigator n in parent.Select ("Note[Reference]")) {
 				Assert.IsFalse (l.Contains (n), "should not iterate the same XPathNavigator instance twice, regardless of whether position is same or not");
 				l.Add (n);
 			}
 
-			// Same test for SimpleSlashIterator.
+			// Same test for SimpleSlashIterator wrapped by XPathNodeIteratorEnumerator.
 			l.Clear ();
 			foreach (XPathNavigator n in data.CreateNavigator ().Select ("/Notes/Note[Reference]")) {
 				Assert.IsFalse (l.Contains (n), "should not iterate the same XPathNavigator instance twice, regardless of whether position is same or not");
 				l.Add (n);
 			}
+
+			// test orderby too. In short, XPathNodeIteratorEnumerator is the key player that assures XPathNavigator uniqueness.
+			foreach (XPathNavigator n in data.CreateNavigator ()
+				.Select ("/Notes/Note[Reference]")
+				.Cast<XPathNavigator> ()
+				.OrderBy (nav => nav.SelectSingleNode ("Reference").Value)) {
+				Assert.IsFalse (l.Contains (n), "should not iterate the same XPathNavigator instance twice, regardless of whether position is same or not");
+				l.Add (n);
+			}
 		}
 	}
 }