fix($q): fix forwarding resolution when callbacks aren't functions

Uses the changes from @jamestalmage's fix in #3535. (thanks!) Closes #3535
angular · Aug 21, 2013 · 7d188d6 · 7d188d6
1 parent 8ee9a3e
commit 7d188d6
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 9 deletions.
diff --git a/src/ng/q.js b/src/ng/q.js
@@ -241,7 +241,7 @@ function qFactory(nextTick, exceptionHandler) {
 
           var wrappedCallback = function(value) {
             try {
-              result.resolve((callback || defaultCallback)(value));
+              result.resolve((isFunction(callback) ? callback : defaultCallback)(value));
             } catch(e) {
               result.reject(e);
               exceptionHandler(e);
@@ -250,7 +250,7 @@ function qFactory(nextTick, exceptionHandler) {
 
           var wrappedErrback = function(reason) {
             try {
-              result.resolve((errback || defaultErrback)(reason));
+              result.resolve((isFunction(errback) ? errback : defaultErrback)(reason));
             } catch(e) {
               result.reject(e);
               exceptionHandler(e);
@@ -259,7 +259,7 @@ function qFactory(nextTick, exceptionHandler) {
 
           var wrappedProgressback = function(progress) {
             try {
-              result.notify((progressback || defaultCallback)(progress));
+              result.notify((isFunction(progressback) ? progressback : defaultCallback)(progress));
             } catch(e) {
               exceptionHandler(e);
             }
@@ -297,7 +297,7 @@ function qFactory(nextTick, exceptionHandler) {
             } catch(e) {
               return makePromise(e, false);
             }
-            if (callbackOutput && callbackOutput.then) {
+            if (callbackOutput && isFunction(callbackOutput.then)) {
               return callbackOutput.then(function() {
                 return makePromise(value, isResolved);
               }, function(error) {
@@ -322,7 +322,7 @@ function qFactory(nextTick, exceptionHandler) {
 
 
   var ref = function(value) {
-    if (value && value.then) return value;
+    if (value && isFunction(value.then)) return value;
     return {
       then: function(callback) {
         var result = defer();
@@ -375,7 +375,7 @@ function qFactory(nextTick, exceptionHandler) {
       then: function(callback, errback) {
         var result = defer();
         nextTick(function() {
-          result.resolve((errback || defaultErrback)(reason));
+          result.resolve((isFunction(errback) ? errback : defaultErrback)(reason));
         });
         return result.promise;
       }
@@ -401,7 +401,7 @@ function qFactory(nextTick, exceptionHandler) {
 
     var wrappedCallback = function(value) {
       try {
-        return (callback || defaultCallback)(value);
+        return (isFunction(callback) ? callback : defaultCallback)(value);
       } catch (e) {
         exceptionHandler(e);
         return reject(e);
@@ -410,7 +410,7 @@ function qFactory(nextTick, exceptionHandler) {
 
     var wrappedErrback = function(reason) {
       try {
-        return (errback || defaultErrback)(reason);
+        return (isFunction(errback) ? errback : defaultErrback)(reason);
       } catch (e) {
         exceptionHandler(e);
         return reject(e);
@@ -419,7 +419,7 @@ function qFactory(nextTick, exceptionHandler) {
 
     var wrappedProgressback = function(progress) {
       try {
-        return (progressback || defaultCallback)(progress);
+        return (isFunction(progressback) ? progressback : defaultCallback)(progress);
       } catch (e) {
         exceptionHandler(e);
       }

diff --git a/test/ng/qSpec.js b/test/ng/qSpec.js
@@ -730,6 +730,38 @@ describe('q', function() {
           mockNextTick.flush();
           expect(log).toEqual(['error(oops!)->reject(oops!)']);
         });
+
+        it('should forward success resolution when success callbacks are not functions', function() {
+          deferred.resolve('yay!');
+
+          promise.then(1).
+                  then(null).
+                  then({}).
+                  then('gah!').
+                  then([]).
+                  then(success());
+
+          expect(logStr()).toBe('');
+
+          mockNextTick.flush();
+          expect(log).toEqual(['success(yay!)->yay!']);
+        });
+
+        it('should forward error resolution when error callbacks are not functions', function() {
+          deferred.reject('oops!');
+
+          promise.then(null, 1).
+                  then(null, null).
+                  then(null, {}).
+                  then(null, 'gah!').
+                  then(null, []).
+                  then(null, error());
+
+          expect(logStr()).toBe('');
+
+          mockNextTick.flush();
+          expect(log).toEqual(['error(oops!)->reject(oops!)']);
+        });
       });