Closed
Bug 899110
Opened 11 years ago
Closed 11 years ago
Remove the code to switch between different back-ends from the Downloads Panel
Categories
(Firefox :: Downloads Panel, defect)
Firefox
Downloads Panel
Tracking
()
RESOLVED
FIXED
Firefox 28
People
(Reporter: Paolo, Assigned: Paolo)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
The preference introduced in bug 899107 should be removed, and the code that
uses nsIDownloadManager in the Downloads Panel should be deleted as well.
Assignee | ||
Comment 1•11 years ago
|
||
This bug will remove the code, while bug 928349 removes the preference.
Component: Download Manager → Downloads Panel
Depends on: 928349
Product: Toolkit → Firefox
Summary: Remove the preference to switch between different back-ends for the Downloads Panel → Remove the code to switch between different back-ends from the Downloads Panel
Assignee | ||
Comment 2•11 years ago
|
||
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #821721 -
Flags: review?(enndeakin)
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 821721 [details] [diff] [review]
The patch
Review of attachment 821721 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/components/downloads/src/DownloadsCommon.jsm
@@ -1176,5 @@
> - // TODO Bug 830415: this isn't the right place to set these annotation.
> - // It should be set it in places' nsIDownloadHistory implementation.
> - if (!this._isPrivate && !dataItem.inProgress) {
> - let downloadMetaData = { state: dataItem.state,
> - endTime: dataItem.endTime };
I've commented in bug 830415 about this code, that we don't execute anymore since we switched to the JavaScript API for downloads. This shouldn't stop us from removing this currently unused code, anyways.
Comment 4•11 years ago
|
||
Comment on attachment 821721 [details] [diff] [review]
The patch
- get visible()
- {
- // If we're still using the toolkit downloads manager, delegate the call
- // to it. Otherwise, return true for now, until we decide on how we want
- // to indicate that a new download has started if a browser window is
- // not available or minimized.
- return DownloadsCommon.useToolkitUI ? this._toolkitUI.visible : true;
- },
+ get visible() true,
The comment suggests that we don't always want to return true. Is that no longer the case?
Attachment #821721 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Neil Deakin from comment #4)
> Comment on attachment 821721 [details] [diff] [review]
> The patch
>
> - get visible()
> - {
> - // If we're still using the toolkit downloads manager, delegate the call
> - // to it. Otherwise, return true for now, until we decide on how we want
> - // to indicate that a new download has started if a browser window is
> - // not available or minimized.
> - return DownloadsCommon.useToolkitUI ? this._toolkitUI.visible : true;
> - },
> + get visible() true,
>
> The comment suggests that we don't always want to return true. Is that no
> longer the case?
Exactly, the return value is irrelevant now that we use the JavaScript API.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
You need to log in
before you can comment on or make changes to this bug.
Description
•