Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Description • 11 years ago

for resolve and reject, no arg == passing undefined.
then(null, function() {}) is used in a lot of code when catch() wasn't part of the standard. The promises-unwrapping spec also makes a IsCallable() check, but does not error.
Similarly catch(null) is dumb, but allowed.

	Domenic Denicola
	Comment 1 • 11 years ago

Notably, `promise.then()` gives you a "clone" of `promise`.

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 11 years ago

Are you actually talking about "no arg" or "null passed"?  Those are different things...

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Comment 3 • 11 years ago

(In reply to Boris Zbarsky [:bz] from comment #2)
> Are you actually talking about "no arg" or "null passed"?  Those are
> different things...

I intend to fix both things, by changing stuff to |optional any| (resolve/reject) or |optional AnyCallback?| (then/catch)

	Andrea Marchesini [:baku]
	Comment 5 • 11 years ago

Comment on attachment 8334061 [details] [diff] [review]
Make Promise.resolve(), Promise.reject(), Promise.prototype.then() and Promise.prototype.catch() spec compliant.

Review of attachment 8334061 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/promise/Promise.cpp
@@ +415,5 @@
>  }
>  
>  /* static */ already_AddRefed<Promise>
>  Promise::Resolve(const GlobalObject& aGlobal, JSContext* aCx,
> +                 const Optional<JS::Handle<JS::Value> >& aValue, ErrorResult& aRv)

we can remove this additional space Optional<JS::Handle<JS::Value>>& aValue.
Here and everywhere else.

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Comment 6 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/58f2fbf00c8c

	Andrea Marchesini [:baku]
	Comment 7 • 11 years ago

What about workers? Do you test all of these methods in workers too?

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?) Assignee
	Comment 8 • 11 years ago

(In reply to Andrea Marchesini (:baku) from comment #7)
> What about workers? Do you test all of these methods in workers too?

The workers test in Bug 915233 tests these methods, I can update it to have the 4-5 new tests added here. There isn't major change, so it should work.

	Carsten Book [:Tomcat]
	Comment 9 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/58f2fbf00c8c