From b57dbf1cdef5ec8e4c087256ee84b3fa807c189b Mon Sep 17 00:00:00 2001
From: Caitlin Potter <caitpotter88@gmail.com>
Date: Tue, 9 Dec 2014 10:42:09 -0500
Subject: [PATCH] fix(orderBy): do not try to call valueOf/toString on `null`

8bfeddb5d671017f4a21b8b46334ac816710b143 added changes to make relational operator work as it
normally would in JS --- unfortunately, this broke due to my failure to account for typeof null
being "object".

This refactoring attempts to convert object values to primitives still, in a fashion similar to
the SortCompare (and subsequently the ToString() algorithm) from ES, in order to account for `null`
and also simplify code to some degree.

BREAKING CHANGE:

Previously, if either value being compared in the orderBy comparator was null or undefined, the
order would not change. Now, this order behaves more like Array.prototype.sort, which by default
pushes `null` behind objects, due to `n` occurring after `[` (the first characters of their
stringified forms) in ASCII / Unicode. If `toString` is customized, or does not exist, the
behaviour is undefined.

Closes #10385
---
 src/ng/filter/orderBy.js      | 46 ++++++++++++++++++++---------------
 test/ng/filter/orderBySpec.js | 32 ++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 19 deletions(-)

diff --git a/src/ng/filter/orderBy.js b/src/ng/filter/orderBy.js
index 3b4d11beb394..8ae08eb81dc6 100644
--- a/src/ng/filter/orderBy.js
+++ b/src/ng/filter/orderBy.js
@@ -160,29 +160,37 @@ function orderByFilter($parse) {
           ? function(a, b) {return comp(b,a);}
           : comp;
     }
+
+    function isPrimitive(value) {
+      switch (typeof value) {
+        case 'number': /* falls through */
+        case 'boolean': /* falls through */
+        case 'string':
+          return true;
+        default:
+          return false;
+      }
+    }
+
+    function objectToString(value) {
+      if (value === null) return 'null';
+      if (typeof value.toString === 'function') {
+        value = value.toString();
+        if (isPrimitive(value)) return value;
+      }
+      if (typeof value.valueOf === 'function') {
+        value = value.valueOf();
+        if (isPrimitive(value)) return value;
+      }
+      return '';
+    }
+
     function compare(v1, v2) {
       var t1 = typeof v1;
       var t2 = typeof v2;
-      // Prepare values for Abstract Relational Comparison
-      // (http://www.ecma-international.org/ecma-262/5.1/#sec-11.8.5):
-      // If the resulting values are identical, return 0 to prevent
-      // incorrect re-ordering.
       if (t1 === t2 && t1 === "object") {
-        // If types are both numbers, emulate abstract ToPrimitive() operation
-        // in order to get primitive values suitable for comparison
-        t1 = typeof (v1.valueOf ? v1 = v1.valueOf() : v1);
-        t2 = typeof (v2.valueOf ? v2 = v2.valueOf() : v2);
-        if (t1 === t2 && t1 === "object") {
-          // Object.prototype.valueOf will return the original object, by
-          // default. If we do not receive a primitive value, use ToString()
-          // instead.
-          t1 = typeof (v1.toString ? v1 = v1.toString() : v1);
-          t2 = typeof (v2.toString ? v2 = v2.toString() : v2);
-
-          // If the end result of toString() for each item is the same, do not
-          // perform relational comparison, and do not re-order objects.
-          if (t1 === t2 && v1 === v2 || t1 === "object") return 0;
-        }
+        v1 = objectToString(v1);
+        v2 = objectToString(v2);
       }
       if (t1 === t2) {
         if (t1 === "string") {
diff --git a/test/ng/filter/orderBySpec.js b/test/ng/filter/orderBySpec.js
index 8380b7a048c2..120425ba78ea 100644
--- a/test/ng/filter/orderBySpec.js
+++ b/test/ng/filter/orderBySpec.js
@@ -125,6 +125,22 @@ describe('Filter: orderBy', function() {
       });
       expect(orderBy(array)).toEqualData(array);
     });
+
+
+    it('should sort nulls as Array.prototype.sort', function() {
+      var array = [
+        { id: 2 },
+        null,
+        { id: 3 },
+        null
+      ];
+      expect(orderBy(array)).toEqualData([
+        { id: 2 },
+        { id: 3 },
+        null,
+        null
+      ]);
+    });
   });
 
 
@@ -252,5 +268,21 @@ describe('Filter: orderBy', function() {
       });
       expect(orderBy(array)).toEqualData(array);
     });
+
+
+    it('should sort nulls as Array.prototype.sort', function() {
+      var array = [
+      { id: 2 },
+      null,
+      { id: 3 },
+      null
+      ];
+      expect(orderBy(array)).toEqualData([
+        { id: 2 },
+        { id: 3 },
+        null,
+        null
+      ]);
+    });
   });
 });