Closed
Bug 1245640
Opened 8 years ago
Closed 8 years ago
Implement chrome.downloads.showDefaultFolder()
Categories
(WebExtensions :: Untriaged, defect)
WebExtensions
Untriaged
Tracking
(firefox48 fixed)
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
Updated•8 years ago
|
Whiteboard: [downloads]
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → aswan
Iteration: --- → 48.1 - Mar 21
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/38781/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/38781/
Attachment #8728101 -
Flags: review?(kmaglione+bmo)
Assignee | ||
Comment 2•8 years ago
|
||
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?
Assignee | ||
Comment 3•8 years ago
|
||
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)
Comment 4•8 years ago
|
||
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()`.
Assignee | ||
Comment 5•8 years ago
|
||
Thanks for the tip. Any thoughts on automated (cross platform) testing?
Flags: needinfo?(kmaglione+bmo)
Comment 6•8 years ago
|
||
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)
Assignee | ||
Updated•8 years ago
|
Attachment #8728101 -
Attachment is obsolete: false
Attachment #8728101 -
Flags: review?(kmaglione+bmo)
Assignee | ||
Comment 7•8 years ago
|
||
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/
Assignee | ||
Comment 8•8 years ago
|
||
Tested manually on osx. Stand by for manual testing on linux and windows once I get good try builds...
Comment 9•8 years ago
|
||
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.
Comment 10•8 years ago
|
||
(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.
Assignee | ||
Comment 11•8 years ago
|
||
Manually verified on Ubuntu (14.04, 64 bit) and Windows (8.1)
Comment 12•8 years ago
|
||
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+
Assignee | ||
Comment 13•8 years ago
|
||
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/
Assignee | ||
Comment 14•8 years ago
|
||
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/
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/1a08b658924a
Keywords: checkin-needed
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1a08b658924a
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•