fix(scope): $watch (and angular.equals) should support NaN values

- since NaN !== NaN in javascript digest can get into an infinite loop
  when model value is set to NaN
- angular.equals(NaN, NaN) should return true since that's what we
  expect when comparing primitives or objects containing NaN values

Previously NaN because of its special === properties was used as the
initial value for watches, but that results in issues when NaN is used
as model value.

In order to allow for model to be anything incuding undefined and NaN we
need to mark the initial value differently in a way that would avoid
these issues, allow us to run digest without major perf penalties and
allow for clients to determine if the listener is being called because
the watcher is being initialized or because the model changed. This
implementation covers all of these scenarios.

BREAKING CHANGE: previously to detect if the listener was called because
the watcher was being initialized, it was suggested that clients check
if old value is NaN. With this change, the check should be if the newVal
equals the oldVal.

Closes #657

5 files changed, 61 insertions, 7 deletions

diff --git a/src/Angular.js b/src/Angular.js
index 127be95d..e5f35bba 100644
--- a/src/Angular.js
+++ b/src/Angular.js
@@ -624,6 +624,7 @@ function copy(source, destination){
  *
  * * Both objects or values pass `===` comparison.
  * * Both objects or values are of the same type and all of their properties pass `===` comparison.
+ * * Both values are NaN. (In JavasScript, NaN == NaN => false. But we consider two NaN as equal)
  *
  * During a property comparision, properties of `function` type and properties with names
  * that begin with `$` are ignored.
@@ -634,11 +635,11 @@ function copy(source, destination){
  * @param {*} o1 Object or value to compare.
  * @param {*} o2 Object or value to compare.
  * @returns {boolean} True if arguments are equal.
- *
  */
 function equals(o1, o2) {
   if (o1 === o2) return true;
   if (o1 === null || o2 === null) return false;
+  if (o1 !== o1 && o2 !== o2) return true; // NaN === NaN
   var t1 = typeof o1, t2 = typeof o2, length, key, keySet;
   if (t1 == t2 && t1 == 'object') {
     if (isArray(o1)) {
diff --git a/src/directives.js b/src/directives.js
index da540df5..41fbac16 100644
--- a/src/directives.js
+++ b/src/directives.js
@@ -576,7 +576,9 @@ function ngClass(selector) {
     return function(element) {
       this.$watch(expression, function(scope, newVal, oldVal) {
         if (selector(scope.$index)) {
-          if (oldVal) element.removeClass(isArray(oldVal) ? oldVal.join(' ') : oldVal);
+          if (oldVal && (newVal !== oldVal)) {
+            element.removeClass(isArray(oldVal) ? oldVal.join(' ') : oldVal);
+          }
           if (newVal) element.addClass(isArray(newVal) ? newVal.join(' ') : newVal);
         }
       });
@@ -823,7 +825,9 @@ angularDirective("ng:hide", function(expression, element){
 angularDirective("ng:style", function(expression, element) {
   return function(element) {
     this.$watch(expression, function(scope, newStyles, oldStyles) {
-      if (oldStyles) forEach(oldStyles, function(val, style) { element.css(style, '');});
+      if (oldStyles && (newStyles !== oldStyles)) {
+        forEach(oldStyles, function(val, style) { element.css(style, '');});
+      }
       if (newStyles) element.css(newStyles);
     });
   };
diff --git a/src/service/scope.js b/src/service/scope.js
index df969cfc..cbda4495 100644
--- a/src/service/scope.js
+++ b/src/service/scope.js
@@ -187,7 +187,8 @@ function $RootScopeProvider(){
        *   reruns when it detects changes the `watchExpression` can execute multiple times per
        *   {@link angular.module.ng.$rootScope.Scope#$digest $digest()} and should be idempotent.)
        * - The `listener` is called only when the value from the current `watchExpression` and the
-       *   previous call to `watchExpression' are not equal. The inequality is determined according to
+       *   previous call to `watchExpression' are not equal (with the exception of the initial run
+       *   see below). The inequality is determined according to
        *   {@link angular.equals} function. To save the value of the object for later comparison
        *   {@link angular.copy} function is used. It also means that watching complex options will
        *   have adverse memory and performance implications.
@@ -201,6 +202,13 @@ function $RootScopeProvider(){
        * can execute multiple times per {@link angular.module.ng.$rootScope.Scope#$digest $digest} cycle when a change is
        * detected, be prepared for multiple calls to your listener.)
        *
+       * After a watcher is registered with the scope, the `listener` fn is called asynchronously
+       * (via {@link angular.module.ng.$rootScope.Scope#$evalAsync $evalAsync}) to initialize the
+       * watcher. In rare cases, this is undesirable because the listener is called when the result
+       * of `watchExpression` didn't change. To detect this scenario within the `listener` fn, you
+       * can compare the `newVal` and `oldVal`. If these two values are identical (`===`) then the
+       * listener was called due to initialization.
+       *
        *
        * # Example
          <pre>
@@ -244,7 +252,7 @@ function $RootScopeProvider(){
             array = scope.$$watchers,
             watcher = {
               fn: listenFn,
-              last: Number.NaN, // NaN !== NaN. We used this to force $watch to fire on first run.
+              last: initWatchVal,
               get: get,
               exp: watchExp
             };
@@ -345,7 +353,7 @@ function $RootScopeProvider(){
                   if ((value = watch.get(current)) !== (last = watch.last) && !equals(value, last)) {
                     dirty = true;
                     watch.last = copy(value);
-                    watch.fn(current, value, last);
+                    watch.fn(current, value, ((last === initWatchVal) ? value : last));
                     if (ttl < 5) {
                       if (!watchLog[4-ttl]) watchLog[4-ttl] = [];
                       if (isFunction(watch.exp)) {
@@ -669,6 +677,10 @@ function $RootScopeProvider(){
       return fn;
     }
 
-
+    /**
+     * function used as an initial value for watchers.
+     * because it's uniqueue we can easily tell it apart from other values
+     */
+    function initWatchVal() {}
   }];
 }
diff --git a/test/AngularSpec.js b/test/AngularSpec.js
index 81a4901b..53f62f00 100644
--- a/test/AngularSpec.js
+++ b/test/AngularSpec.js
@@ -110,6 +110,10 @@ describe('angular', function() {
 
       expect(equals(undefined, undefined)).toBe(true);
     });
+
+    it('should treat two NaNs as equal', function() {
+      expect(equals(NaN, NaN)).toBe(true);
+    });
   });
 
   describe('size', function() {
diff --git a/test/service/scopeSpec.js b/test/service/scopeSpec.js
index 71a27b33..6e8d78ff 100644
--- a/test/service/scopeSpec.js
+++ b/test/service/scopeSpec.js
@@ -305,6 +305,39 @@ describe('Scope', function() {
       root.$digest(); //trigger
       expect(listener).not.toHaveBeenCalled();
     }));
+
+
+    it('should not infinitely digest when current value is NaN', inject(function($rootScope) {
+      $rootScope.$watch(function() { return NaN;});
+
+      expect(function() {
+        $rootScope.$digest();
+      }).not.toThrow();
+    }));
+
+
+    it('should always call the watchr with newVal and oldVal equal on the first run',
+        inject(function($rootScope) {
+      var log = [];
+      function logger(scope, newVal, oldVal) {
+        var val = (newVal === oldVal || (newVal !== oldVal && oldVal !== newVal)) ? newVal : 'xxx';
+        log.push(val);
+      };
+
+      $rootScope.$watch(function() { return NaN;}, logger);
+      $rootScope.$watch(function() { return undefined;}, logger);
+      $rootScope.$watch(function() { return '';}, logger);
+      $rootScope.$watch(function() { return false;}, logger);
+      $rootScope.$watch(function() { return {};}, logger);
+      $rootScope.$watch(function() { return 23;}, logger);
+
+      $rootScope.$digest();
+      expect(isNaN(log.shift())).toBe(true); //jasmine's toBe and toEqual don't work well with NaNs
+      expect(log).toEqual([undefined, '', false, {}, 23]);
+      log = []
+      $rootScope.$digest();
+      expect(log).toEqual([]);
+    }));
   });