Closed Bug 984872 Opened 8 years ago Closed 8 years ago

Remove deprecated promise.js usage in Firefox for Desktop

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 31

People

(Reporter: Paolo, Assigned: Paolo)

References

Details

Attachments

(1 file)

Attached patch The patchSplinter Review
This patch removes the remaining uses of Add-on SDK Promises in the Firefox for Desktop front-end code.

The only test that needed to be changed was the sanitizer. The production code was logically correct, but the test assumed synchronous processing when clicking the OK button.
Attachment #8392849 - Flags: review?(mak77)
Comment on attachment 8392849 [details] [diff] [review]
The patch

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

r=me, provided a green Try!
Attachment #8392849 - Flags: review?(mak77) → review+
Comment on attachment 8392849 [details] [diff] [review]
The patch

>diff --git a/browser/base/content/sanitizeDialog.js b/browser/base/content/sanitizeDialog.js

>-      s.sanitize().then(window.close, window.close);
>+      s.sanitize().then(null, Components.utils.reportError)
>+                  .then(() => window.close())
>+                  .then(null, Components.utils.reportError);

Won't this result in the window staying open if the promise is rejected? That wasn't the intent of the old code...
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #3)
> Won't this result in the window staying open if the promise is rejected?
> That wasn't the intent of the old code...

As I read it, if the sanitize promise is rejected it goes reportError, but in any case both success or failure go through window.close(). The third then is to handle window.close exceptions.
https://hg.mozilla.org/mozilla-central/rev/86f09e63ac6c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Blocks: 856878
Blocks: 996671
No longer blocks: 856878
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.