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

Status: RESOLVED FIXED

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

Description

[Andrea Marchesini [:baku]]

Basically the specs uses [TreatUndefinedAs=Missing] for any callback in then(), catch(), done().

Comment 1

[Andrea Marchesini [:baku]]

Attachment: part 1 - renaming

Comment 2

[Andrea Marchesini [:baku]]

Attachment: Remove Done() and update then() and catch()

Current the spec says:

1. done() is removed
2. then() and catch() use [TreatUndefinedAs=Missing]

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 761015 [details] [diff] [review]
Remove Done() and update then() and catch()

Please fix the indentation in Promise::Then?

Comment 4

[Andrea Marchesini [:baku]]

Attachment: part 1 - renaming

Comment 5

[Andrea Marchesini [:baku]]

Attachment: part 3 - Fulfill()

Comment 6

[Andrea Marchesini [:baku]]

Attachment: part 2 - fulfill()

revolveCb -> fulfillCb

Comment 7

[Andrea Marchesini [:baku]]

Comment on attachment 761054 [details] [diff] [review]
part 1 - renaming

Make as obsolate because this patch is moved to bug 884279.

Comment 8

[Andrea Marchesini [:baku]]

Comment on attachment 763421 [details] [diff] [review]
part 2 - fulfill()

Moved to bug 884754

Comment 9

[Mounir Lamouri (:mounir)]

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.

Comment 10

[Andrea Marchesini [:baku]]

Attachment: Remove Done() and update then() and catch()

Commit message updated.

Comment 11

[Boris Zbarsky [: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....

Comment 12

[David Bruant]

(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

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8df19e23b3ae

Comment 14

[(no longer active)]

Backed out because this depends on bug 884279 which I also backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e39187b8199

Comment 15

[Andrea Marchesini [:baku]]

Attachment: patch

rebased on top of m-c and m-i

Comment 16

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80732a2b8422

Comment 17

[Ed Morley [:emorley]]

Backed out for mochitest-chrome failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=80732a2b8422

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/2266ccdba6d7
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bf5031737e46

Comment 18

[Andrea Marchesini [:baku]]

Now this issue should

Comment 19

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5ebb29f3ae2

Comment 20

[Andrea Marchesini [:baku]]

I was saying: now this patch is ready to land.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b5ebb29f3ae2