Closed
Bug 851461
Opened 8 years ago
Closed 8 years ago
Make the JavaScript API for downloads available in parallel to nsIDownloadManager
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: Paolo, Assigned: Paolo)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(1 file, 4 obsolete files)
24.09 KB,
patch
|
Details | Diff | Splinter Review |
An initial version of the JavaScript API for downloads should become available to Toolkit consumers before nsIDownloadManager is actually removed, so that add-ons and host application front-ends can start experimenting with it. The API would be present in the background, and would be activated only if an add-on uses it. Ideally, it should be possible to redirect downloads initiated through nsITransfer to the new API using a hidden about:config preference. Even when the preference is not set, consumers that use the new API both to start and to view downloads should be able to see the downloads they start. A preliminary super-review on the overall API calls is welcome before making the API available, and early documentation of the validated API on the Mozilla Developer Network site would also be appropriate at that point.
Assignee | ||
Comment 1•8 years ago
|
||
This enables the JavaScript API for downloads for all Toolkit consumers. This does not affect the behavior of existing consumers, though in Firefox there is now a preference (that is read in the Downloads component on startup for convenience) to redirect legacy downloads to the new API. Together with the patch in bug 847863, this allows downloads to go through the new API and be seen in the Downloads Panel.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #726714 -
Flags: superreview?(gavin.sharp)
Attachment #726714 -
Flags: review?(mak77)
Assignee | ||
Comment 2•8 years ago
|
||
Forgot to update the xpcshell tests to always create the nsITransfer implementation of the JavaScript API. Original and fixed tryserver builds are running here: https://tbpl.mozilla.org/?tree=Try&rev=2942eefaf723 https://tbpl.mozilla.org/?tree=Try&rev=39b42d981741
Attachment #726714 -
Attachment is obsolete: true
Attachment #726714 -
Flags: superreview?(gavin.sharp)
Attachment #726714 -
Flags: review?(mak77)
Attachment #726736 -
Flags: superreview?(gavin.sharp)
Attachment #726736 -
Flags: review?(mak77)
Comment 3•8 years ago
|
||
Comment on attachment 726736 [details] [diff] [review] Updated patch Review of attachment 726736 [details] [diff] [review]: ----------------------------------------------------------------- please reflag when answering questions ::: browser/components/downloads/src/DownloadsStartup.js @@ +32,5 @@ > "@mozilla.org/browser/sessionstartup;1", > "nsISessionStartup"); > > const kObservedTopics = [ > + "profile-after-change", just add category profile-after-change to http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/src/BrowserDownloads.manifest#3 While there, is there a specific reason we need to begin at app-startup compared to just putting everything in profile-after-change? @@ +102,5 @@ > + case "profile-after-change": > + // If the integration preference is enabled, override Toolkit's > + // nsITransfer implementation with the one from the JavaScript API for > + // downloads. This should be used only by developers while testing new > + // code that uses the JavaScript API, and will eventually be removed. add bug number to remove this ::: toolkit/components/jsdownloads/src/Downloads.manifest @@ +1,3 @@ > component {1b4c85df-cbdd-4bb6-b04e-613caece083c} DownloadLegacy.js > + > +# This is disabled since we also build "toolkit/components/downloads" a better comment about what happens and what will happen in future would be more than welcome here. Even with bug numbers.
Attachment #726736 -
Flags: review?(mak77)
Comment 4•8 years ago
|
||
Comment on attachment 726736 [details] [diff] [review] Updated patch Oops, sorry for the delay here.
Attachment #726736 -
Flags: superreview?(gavin.sharp) → superreview+
Assignee | ||
Comment 5•8 years ago
|
||
Attachment #726736 -
Attachment is obsolete: true
Attachment #733235 -
Flags: review?(mak77)
Assignee | ||
Comment 6•8 years ago
|
||
Attachment #733235 -
Attachment is obsolete: true
Attachment #733235 -
Flags: review?(mak77)
Attachment #733241 -
Flags: review?(mak77)
Comment 7•8 years ago
|
||
Comment on attachment 733241 [details] [diff] [review] More robust tests on Windows Review of attachment 733241 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/jsdownloads/test/unit/head.js @@ +107,4 @@ > { > + // Prepend a serial number to the first dot in the suggested leaf name. > + let leafName = aLeafNameWithExtension.replace(".", "-" + gFileCounter + "."); > + gFileCounter++; I wonder if you may use DownloadPaths.jsm utils here ::: toolkit/components/jsdownloads/test/unit/test_DownloadLegacy.js @@ +34,5 @@ > let persist = Cc["@mozilla.org/embedding/browser/nsWebBrowserPersist;1"] > .createInstance(Ci.nsIWebBrowserPersist); > + > + // We must create the nsITransfer implementation using its class ID until we > + // always implement the "@mozilla.org/transfer;1" contract. add bug# where that will happen
Attachment #733241 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #733241 -
Attachment is obsolete: true
Assignee | ||
Comment 9•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/833dc6a7c475
Target Milestone: --- → mozilla23
Comment 10•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/833dc6a7c475
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•