remove DownloadMonitorPanel remnants

RESOLVED FIXED in Firefox 20

Status

()

Firefox
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mak, Assigned: mak)

Tracking

unspecified
Firefox 20
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
I suppose this is a remaining part of the downloads in status bar thing, with the new downlods indicator this unmaintained code is likely pointless.
also the #download-monitor styling.

Gavin, is there a reason this was not removed? does it support some add-on or whatever else? It just looks like dead code to me, the only add-on using it looks like FireTorrent
Flags: needinfo?(gavin.sharp)
(Assignee)

Comment 1

5 years ago
http://mxr.mozilla.org/mozilla-central/search?string=DownloadMonitorPanel
http://mxr.mozilla.org/mozilla-central/search?string=%23download-monitor&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
https://mxr.mozilla.org/addons/search?string=DownloadMonitorPanel&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=addons
Huh, I wasn't aware of the history here. Apparently bug 574688 removed the feature implemented in bug 402278, but only partially? Wonderful!

I don't see any reason to keep this code. http://hg.mozilla.org/mozilla-central/rev/d1ac428674f2 shows that you already identified all of the unused pieces, I think.
Flags: needinfo?(gavin.sharp)
Summary: Evaluate removing DownloadMonitorPanel from browser → remove DownloadMonitorPanel remnants
(Assignee)

Comment 3

5 years ago
sounds like a relaxing bug to take.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
(Assignee)

Comment 4

5 years ago
Created attachment 688719 [details] [diff] [review]
patch v1.0
Attachment #688719 - Flags: review?(dao)
Comment on attachment 688719 [details] [diff] [review]
patch v1.0

>-    this._activeStr = gNavigatorBundle.getString("activeDownloads1");
>-    this._pausedStr = gNavigatorBundle.getString("pausedDownloads1");

These strings are now unused.

>--- a/browser/themes/pinstripe/browser.css
>+++ b/browser/themes/pinstripe/browser.css
>@@ -3125,20 +3125,16 @@ toolbarbutton.chevron > .toolbarbutton-m
> #identity-popup-more-info-button:focus {
>   @hudButtonFocused@
> }
> 
> #identity-popup-more-info-button:hover:active {
>   @hudButtonPressed@
> }
> 
>-#download-monitor {
>-  list-style-image: url("chrome://mozapps/skin/downloads/downloadStatusIcon.png");
>-}

This image is now unused.

This patch makes gDownloadMgr unused as well.
Attachment #688719 - Flags: review?(dao) → review-

Updated

5 years ago
Component: Downloads Panel → General
(Assignee)

Comment 6

5 years ago
ah good, I thought I searched for gDownloadMgr, probably search failed :/

looks like the string have been added in another push:
http://hg.mozilla.org/mozilla-central/rev/e1214959aead
So this is other stuff to check

I couldn't find the original push for the image, looks like has been addes with the new pinstripe in 2008
(Assignee)

Comment 7

5 years ago
and also
http://hg.mozilla.org/mozilla-central/rev/b320791e4dde
(Assignee)

Comment 8

5 years ago
Created attachment 688793 [details] [diff] [review]
patch v1.1

sorry for the incomplete patch on first try!
Attachment #688719 - Attachment is obsolete: true
Attachment #688793 - Flags: review?(dao)

Updated

5 years ago
Attachment #688793 - Flags: review?(dao) → review+
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/33292b24d5d2
Target Milestone: --- → Firefox 20
https://hg.mozilla.org/mozilla-central/rev/33292b24d5d2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.