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

Status: RESOLVED FIXED

(Firefox :: Downloads Panel, defect)

Description

[:Paolo Amadini]

The preference introduced in bug 899107 should be removed, and the code that
uses nsIDownloadManager in the Downloads Panel should be deleted as well.

Comment 1

[:Paolo Amadini]

This bug will remove the code, while bug 928349 removes the preference.

Comment 2

[:Paolo Amadini]

Attachment: The patch

Comment 3

[:Paolo Amadini]

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

[Neil Deakin]

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?

Comment 5

[:Paolo Amadini]

(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.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/92f44bb34cc8

Comment 7

[Ioana (away)]

Verified with bug 928349.