If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Remove the code to switch between different back-ends from the Downloads Panel

RESOLVED FIXED in Firefox 28

Status

()

Firefox
Downloads Panel
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Paolo, Assigned: Paolo)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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)

Updated

4 years ago
Blocks: 908600
(Assignee)

Comment 1

4 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

4 years ago
Created attachment 821721 [details] [diff] [review]
The patch
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #821721 - Flags: review?(enndeakin)
(Assignee)

Comment 3

4 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

4 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

4 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.
https://hg.mozilla.org/mozilla-central/rev/92f44bb34cc8
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28

Updated

4 years ago
Depends on: 939708

Updated

4 years ago
Depends on: 952353

Updated

4 years ago
Keywords: verifyme

Comment 7

4 years ago
Verified with bug 928349.
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.