Closed Bug 899110 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: Downloads Panel, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: Paolo, Assigned: Paolo)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The preference introduced in bug 899107 should be removed, and the code that
uses nsIDownloadManager in the Downloads Panel should be deleted as well.
Blocks: 908600
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
Attached patch The patchSplinter Review
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #821721 - Flags: review?(enndeakin)
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 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+
(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
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Depends on: 939708
Depends on: 952353
Keywords: verifyme
Verified with bug 928349.
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.