Closed Bug 1120235 Opened 7 years ago Closed 7 years ago

promise Resolver [[Call]] should return undefined, but returns resolver itself

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: erights, Assigned: bzbarsky)

Details

Attachments

(1 file)

> var r;
> var p = new Promise(function(rr) { r = rr; });
> r(88) == r;
true
Moving to the DOM component because our Promise implementation hasn't yet been moved to the JS engine (which doesn't yet implement job queues).
Component: JavaScript Engine → DOM
Yay, JSAPI strikes again.  In JSAPI, returning the function itself is the default behavior unless you go out of your way to return something else.  :(
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 8547572 [details] [diff] [review]
Various promise resolve/reject functions should actually return undefined instead of ending up accidentally returning their callee

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

::: dom/promise/tests/mochitest.ini
@@ +4,5 @@
>  [test_promise.html]
>  [test_promise_utils.html]
>  [test_resolve.html]
>  [test_abortable_promise.html]
> +[test_resolver_return_value.html]

Nit: I prefer test lists to be sorted. I'm not sure how abortable promise broke the convention, could you fix that when landing? Thanks!
Attachment #8547572 - Flags: review?(nsm.nikhil) → review+
Done and https://hg.mozilla.org/integration/mozilla-inbound/rev/02fb8f978e91

Thanks for the quick review!
Target Milestone: --- → mozilla38
https://hg.mozilla.org/mozilla-central/rev/02fb8f978e91
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.