Closed
Bug 973239
Opened 11 years ago
Closed 11 years ago
Promise should call executor and handlers with "undefined" as thisArg
Categories
(Toolkit :: Async Tooling, defect)
Toolkit
Async Tooling
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: bbenvie, Assigned: Paolo)
References
Details
Attachments
(1 file)
|
(deleted),
patch
|
bbenvie
:
review+
|
Details | Diff | Splinter Review |
When implementing the Promise constructor, I followed our DOM Promise implementation's lead on setting the receiver in the executor to the newly created promise. Upon reviewing the spec I realized that this is incorrect and it should be set to `undefined`.
|
Assignee | |
Comment 1•11 years ago
|
||
This fixes both the constructor and the "then" handlers.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #8394706 -
Flags: review?(bbenvie)
|
|
Assignee | |
Updated•11 years ago
|
Summary: Promise constructor should call the executor with `undefined` as thisArg → Promise should call executor and handlers with "undefined" as thisArg
|
Reporter | |
Comment 2•11 years ago
|
||
Comment on attachment 8394706 [details] [diff] [review]
The patch
Review of attachment 8394706 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM except for one thing.
::: toolkit/modules/Promise-backend.js
@@ +703,5 @@
> // to determine the state of the next promise, that will be resolved with
> // the returned value, that can also be another promise.
> if (nextStatus == STATUS_RESOLVED) {
> if (typeof(this.onResolve) == "function") {
> + nextValue = this.onResolve.call(undefined, nextValue);
Either these two should use Function.prototype.call.call or the constructor's call of aExecutor above should just use .call as it is used here.
Attachment #8394706 -
Flags: review?(bbenvie) → review+
|
|
Assignee | |
Comment 3•11 years ago
|
||
Converted to aExecutor.call:
https://hg.mozilla.org/integration/fx-team/rev/b8565a96db37
|
||
Comment 4•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•