Remove .done() and allow undefined to be passed to .then() and .catch()

RESOLVED FIXED in mozilla25

Status

()

defect
RESOLVED FIXED
6 years ago
4 months ago

People

(Reporter: baku, Assigned: baku)

Tracking

(Blocks 2 bugs)

Trunk
mozilla25
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

Basically the specs uses [TreatUndefinedAs=Missing] for any callback in then(), catch(), done().
Blocks: 871445
Posted patch part 1 - renaming (obsolete) — Splinter Review
Attachment #760977 - Flags: review?(mounir)
Summary: Update Future.webIDL to the new specs → Update Promise.webIDL to the new specs
Current the spec says:

1. done() is removed
2. then() and catch() use [TreatUndefinedAs=Missing]
Attachment #761015 - Flags: review?(mounir)
Comment on attachment 761015 [details] [diff] [review]
Remove Done() and update then() and catch()

Please fix the indentation in Promise::Then?
Posted patch part 1 - renaming (obsolete) — Splinter Review
Attachment #760977 - Attachment is obsolete: true
Attachment #760977 - Flags: review?(mounir)
Attachment #761054 - Flags: review?(mounir)
Summary: Update Promise.webIDL to the new specs → Update done/then/catch in Promise.webIDL to the new specs
Posted patch part 3 - Fulfill() (obsolete) — Splinter Review
Attachment #761078 - Flags: review?(mounir)
Summary: Update done/then/catch in Promise.webIDL to the new specs → Update done/then/catch/fulfill in Promise.webIDL to the new specs
Blocks: 882076
Blocks: 882077
Blocks: 879245
Blocks: 839838
Posted patch part 2 - fulfill() (obsolete) — Splinter Review
revolveCb -> fulfillCb
Attachment #761078 - Attachment is obsolete: true
Attachment #761078 - Flags: review?(mounir)
Attachment #763421 - Flags: review?(mounir)
Attachment #763421 - Attachment description: patch → part 3 - fulfill()
Comment on attachment 761054 [details] [diff] [review]
part 1 - renaming

Make as obsolate because this patch is moved to bug 884279.
Attachment #761054 - Attachment is obsolete: true
Attachment #761054 - Flags: review?(mounir)
Attachment #761015 - Attachment description: part 2 - Remove Done() and update then() and catch() → part 1 - Remove Done() and update then() and catch()
Attachment #763421 - Attachment description: part 3 - fulfill() → part 2 - fulfill()
Comment on attachment 763421 [details] [diff] [review]
part 2 - fulfill()

Moved to bug 884754
Attachment #763421 - Attachment is obsolete: true
Attachment #763421 - Flags: review?(mounir)
Depends on: 884754
Attachment #761015 - Attachment description: part 1 - Remove Done() and update then() and catch() → Remove Done() and update then() and catch()
No longer depends on: 884754
Blocks: 884754
Summary: Update done/then/catch/fulfill in Promise.webIDL to the new specs → Remove .done() and allow undefined to be passed to .then() and .catch()
Comment on attachment 761015 [details] [diff] [review]
Remove Done() and update then() and catch()

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

Andrea, could you update the commit summary so it means something? Updating to specs isn't very informative because you could have dozens of commit like that.

Asking bz to sr because this is a web facing change.
Attachment #761015 - Flags: superreview?(bzbarsky)
Attachment #761015 - Flags: review?(mounir)
Attachment #761015 - Flags: review+
Commit message updated.
Attachment #761015 - Attachment is obsolete: true
Attachment #761015 - Flags: superreview?(bzbarsky)
Attachment #764731 - Flags: superreview?(bzbarsky)
Comment on attachment 764731 [details] [diff] [review]
Remove Done() and update then() and catch()

sr=me, but I think the removal of done() is a mistake.  I said so on the mailing list, and was ignored....
Attachment #764731 - Flags: superreview?(bzbarsky) → superreview+
Keywords: checkin-needed
(In reply to Boris Zbarsky (:bz) from comment #11)
> Comment on attachment 764731 [details] [diff] [review]
> Remove Done() and update then() and catch()
> 
> sr=me, but I think the removal of done() is a mistake.  I said so on the
> mailing list, and was ignored....
I've been an early fighter to get it in the spec and spent lengthy discussions on it too :-( Many other people have expressed their disappointment about that [1].
I guess it's a matter of a big enough group to be convinced of the necessity. MarkM said it best:
"DOMPromises only needs be approximately the minimum we need quick agreement on, so that we can add the rest of what's needed in the ES7 process." [2]

I really wished .done would have been part of the first cut, but meh... it'll come back, I'm sure.

[1] https://mail.mozilla.org/pipermail/es-discuss/2013-June/031396.html
[2] https://mail.mozilla.org/pipermail/es-discuss/2013-June/031426.html
https://hg.mozilla.org/integration/mozilla-inbound/rev/8df19e23b3ae
Assignee: nobody → amarchesini
Flags: in-testsuite+
Keywords: checkin-needed
Backed out because this depends on bug 884279 which I also backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e39187b8199
Posted patch patchSplinter Review
rebased on top of m-c and m-i
Attachment #764731 - Attachment is obsolete: true
Keywords: checkin-needed
Now this issue should
Depends on: 885318
I was saying: now this patch is ready to land.
https://hg.mozilla.org/mozilla-central/rev/b5ebb29f3ae2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.