1247791 - Implement options.saveAs for chrome.downloads.download()

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P2)

Description

[Andrew Swan [:aswan]]

https://developer.chrome.com/extensions/downloads#method-download

Comment 1

[Tomislav Jovanovic :zombie]

Attachment: Bug 1247791 - Implement options.saveAs for downloads.download,

Review commit: https://reviewboard.mozilla.org/r/79572/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79572/

Comment 2

[Tomislav Jovanovic :zombie]

Comment on attachment 8792654 [details]
Bug 1247791 - Implement options.saveAs for downloads.download,

https://reviewboard.mozilla.org/r/79572/#review78236

::: toolkit/components/extensions/ext-downloads.js:462
(Diff revision 1)
> +              return Promise.resolve(target);
> +            }
> +
> +            // Setup the file picker Save As dialog.
> +            const picker = Cc["@mozilla.org/filepicker;1"].createInstance(Ci.nsIFilePicker);
> +            picker.init(global.WindowManager.topWindow, null, picker.modeSave);

eslint complains without the global

Comment 3

[Tomislav Jovanovic :zombie]

Comment on attachment 8792654 [details]
Bug 1247791 - Implement options.saveAs for downloads.download,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79572/diff/1-2/

Comment 4

[Kris Maglione [:kmag]]

Comment on attachment 8792654 [details]
Bug 1247791 - Implement options.saveAs for downloads.download,

https://reviewboard.mozilla.org/r/79572/#review80122

::: toolkit/components/extensions/ext-downloads.js:472
(Diff revision 2)
> +
> +            // Open the dialog and resolve/reject with the result.
> +            return new Promise((resolve, reject) => {
> +              picker.open(result => {
> +                if (result === Ci.nsIFilePicker.returnCancel) {
> +                  reject({message: "download canceled by the user"});

Please capitalize.

Comment 5

[Tomislav Jovanovic :zombie]

Comment on attachment 8792654 [details]
Bug 1247791 - Implement options.saveAs for downloads.download,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79572/diff/2-3/

Comment 6

[Tomislav Jovanovic :zombie]

Comment on attachment 8792654 [details]
Bug 1247791 - Implement options.saveAs for downloads.download,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79572/diff/3-4/

Comment 7

[Tomislav Jovanovic :zombie]

Comment on attachment 8792654 [details]
Bug 1247791 - Implement options.saveAs for downloads.download,

https://reviewboard.mozilla.org/r/79572/#review80122

> Please capitalize.

I was going by the format of reject messages a few lines above:
http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ext-downloads.js#408

Comment 8

[Tomislav Jovanovic :zombie]

Comment on attachment 8792654 [details]
Bug 1247791 - Implement options.saveAs for downloads.download,

https://reviewboard.mozilla.org/r/79572/#review80122

> I was going by the format of reject messages a few lines above:
> http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ext-downloads.js#408

(though now I understand those were property names)

Comment 9

[Kris Maglione [:kmag]]

Comment on attachment 8792654 [details]
Bug 1247791 - Implement options.saveAs for downloads.download,

https://reviewboard.mozilla.org/r/79572/#review80122

> (though now I understand those were property names)

Ah. We should probably fix those too...

Comment 10

[Tomislav Jovanovic :zombie]

(In reply to Kris Maglione [:kmag] from comment #9)
> > (though now I understand those were property names)
> 
> Ah. We should probably fix those too...

this miss-quoting is a result posting comments at almost the same time. Kris doesn't think we should capitalize property names (from irc).

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c017d2d3ffe2
Implement options.saveAs for downloads.download, r=kmag

Comment 12

[Carsten Book [:Tomcat]]

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

Comment 13

[Will Bamberg [:wbamberg]]

Updated compat data:

-> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/downloads/download#Compatibility_notes

Marking dev-doc-complete, but let me know if I need anything else.