Closed Bug 1245640 Opened 8 years ago Closed 8 years ago

Implement chrome.downloads.showDefaultFolder()

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Iteration:
48.1 - Mar 21
Tracking Status
firefox48 --- fixed

People

(Reporter: aswan, Assigned: aswan)

References

(Blocks 1 open bug)

Details

(Whiteboard: [downloads])

Attachments

(1 file)

Part of supporting chrome.downloads in WebExtensions

https://developer.chrome.com/extensions/downloads#method-showDefaultFolder
Blocks: 1213445
Whiteboard: [downloads]
Assignee: nobody → aswan
Iteration: --- → 48.1 - Mar 21
About that review request, this should really be f? at this point -- I tested manually on a Mac, but looking for feedback about how to do automated cross-platform testing?
Comment on attachment 8728101 [details]
MozReview Request: Bug 1245640 - Implement chrome.downloads.showDefaultDirectory() r?kmag

Bah, I'm getting ahead of myself, on osx where the default downloads directory is ~/Downloads, this actually brings up a finder window on ~ with Downloads highlighted, rather than opening a window on ~/Downloads.  Time to take this one back to the drawing board...
Attachment #8728101 - Attachment is obsolete: true
Attachment #8728101 - Flags: review?(kmaglione+bmo)
Yup. If you want to open a directory, use `.launch()` (after making sure it is, in fact, a directory). If you want to show a specific file in a directory, use `.reveal()`.
Thanks for the tip.
Any thoughts on automated (cross platform) testing?
Flags: needinfo?(kmaglione+bmo)
You're going to have a really hard time testing this. For the time being, I'd say test manually, but don't bother with automated testing.

I can't think of a reasonable way to test this, and I can't even find any tests where we test anything similar, so I think we may just have to live with this remaining untested.
Flags: needinfo?(kmaglione+bmo)
Attachment #8728101 - Attachment is obsolete: false
Attachment #8728101 - Flags: review?(kmaglione+bmo)
Comment on attachment 8728101 [details]
MozReview Request: Bug 1245640 - Implement chrome.downloads.showDefaultDirectory() r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38781/diff/1-2/
Tested manually on osx.  Stand by for manual testing on linux and windows once I get good try builds...
I've been meeting with Krupa, Dave and John (all added to cc) about doing some integration tests for web extensions APIs using marionette, which would involve starting an actual browser and exercising APIs. We are particularly interested in areas of the APIs that are not covered via unit tests, so perhaps this is a candidate for that.
(In reply to Bob Silverberg [:bsilverberg] from comment #9)
> I've been meeting with Krupa, Dave and John (all added to cc) about doing
> some integration tests for web extensions APIs using marionette, which would
> involve starting an actual browser and exercising APIs.

I'm not sure what you mean by "starting an actual browser". Our mochitests run
in an actual browser (technically xpcshell, but that's an implementation
detail). What would be different about the Marionette tests?

> We are particularly interested in areas of the APIs that are not covered via
> unit tests, so perhaps this is a candidate for that.

That would be great. I'd be much more comfortable knowing we had some sort of
coverage for this API.
Manually verified on Ubuntu (14.04, 64 bit) and Windows (8.1)
Comment on attachment 8728101 [details]
MozReview Request: Bug 1245640 - Implement chrome.downloads.showDefaultDirectory() r?kmag

https://reviewboard.mozilla.org/r/38781/#review35689

::: toolkit/components/extensions/ext-downloads.js:459
(Diff revision 2)
> +          }

This would be better as a `.catch()` clause.
Attachment #8728101 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8728101 [details]
MozReview Request: Bug 1245640 - Implement chrome.downloads.showDefaultDirectory() r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38781/diff/2-3/
Comment on attachment 8728101 [details]
MozReview Request: Bug 1245640 - Implement chrome.downloads.showDefaultDirectory() r?kmag

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38781/diff/3-4/
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1a08b658924a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: