330139 - Change 'Remove' in download manager to 'Remove From List'

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Jez]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

This should be done for clarification of what clicking on this link will actually do; 'Remove' kinda suggests that it might remove the file from the hard drive, and is ambiguous at best.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

(see bug 327025 comment 8 through comment 13 for background on this bug)

Comment 2

[Harald]

Right! "Clean Up" and "Remove" have the same function. "Remove" doesn't remove the downloaded file from the harddrive.

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

(taking)

Comment 4

[zak]

1.5.0.6 on Linux, clicking "remove" in the download manager actually removes the file from the drive. This is both unexpected and dangerous in my opinion. I expect a browser, under usual operation and without getting tangled up in semantic overkill, to be more-or-less a read-only instrument. I had finished going through about 20 "remove" operations before I realised, to my horror, what was happening. I have no way of knowing what I had deleted via this mechanism. My need to do them on a one-by-one basis was out of a fear that "clean up" might remove entries in the list that had not completed and I want to ge through and re-download those that had not finished. 

Comment 5

[zak]

(In reply to comment #4)
> 1.5.0.6 on Linux, clicking "remove" in the download manager actually removes
> the file from the drive. This is both unexpected and dangerous in my opinion. I
> expect a browser, under usual operation and without getting tangled up in
> semantic overkill, to be more-or-less a read-only instrument. I had finished
> going through about 20 "remove" operations before I realised, to my horror,
> what was happening. I have no way of knowing what I had deleted via this
> mechanism. My need to do them on a one-by-one basis was out of a fear that
> "clean up" might remove entries in the list that had not completed and I want
> to ge through and re-download those that had not finished. 
> 

My apologies, I see that what happened was more subtle. Clicking "remove" did not remove completed files, only those that Firefox thought were not completely downloaded. So I was going through the list, and using wget to regrab the files, then removing from the download list, which then deleted the files off the drive. Still unpredictable behaviour, but not nearly as catastrophic (in this one case) as I had feared.

Comment 6

[Jez]

I still don't like it.  I'd still like to see it changed to 'Clear', but any files listed in Downloads that aren't complete should be suffixed with something like "(not completed)", so the user knows they're leaving an uncompleted file on their system.

Comment 7

[Joey Minta]

Attachment: change entity

Stealing the bug since this didn't seem to make the 2.0 train.  I'll let beltzner review though since it was his originally.

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #7)
> Created an attachment (id=265591) [details]
> change entity
> 
> Stealing the bug since this didn't seem to make the 2.0 train.  I'll let
> beltzner review though since it was his originally.

It'll probably take more than this patch, since bug 388517 removed the button.

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #8)
> (In reply to comment #7)
> > Created an attachment (id=265591) [details] [details]
> > change entity
> > 
> > Stealing the bug since this didn't seem to make the 2.0 train.  I'll let
> > beltzner review though since it was his originally.
> 
> It'll probably take more than this patch, since bug 388517 removed the button.

Nevermind, you're talking about the context menu now, since the actual links are gone; did you want to morph this bug to cover the context menu?

Comment 10

[Shawn Wilsher :sdwilsh]

Comment on attachment 265591 [details] [diff] [review]
change entity

(In reply to comment #9)
> Nevermind, you're talking about the context menu now, since the actual links
> are gone; did you want to morph this bug to cover the context menu?
Yes.

Joey, please feel free to update your patch so it can make it for Firefox 3.

Comment 11

[u88484]

Looks like Joey isn't following this anymore, taking and will post patch in an hour or two.

Comment 12

[u88484]

Attachment: Patch

Changes 'Remove' to 'Clear'

I also changed the menuitem id in both the downloads.xul/.js files. I did not change the key_remove and cmd_remove though. If that is wanted, just let me know.

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

http://people.mozilla.com/~madhava/files/downloadmanager/downloadmanager_2007-09-25_revision.png

According to that, it should read "Remove From List"

Comment 14

[u88484]

Attachment: Patch 2.0

Thanks Stephen.

Patch changes 'Remove' to 'Remove From List'

Comment 15

[Shawn Wilsher :sdwilsh]

(In reply to comment #12)
> I also changed the menuitem id in both the downloads.xul/.js files. I did not
> change the key_remove and cmd_remove though. If that is wanted, just let me
> know.
I would like to see this change please.

Comment 16

[u88484]

Attachment: Patch 3.0

also changes key_remove and cmd_remove to 'removeFromList'

Comment 17

[Shawn Wilsher :sdwilsh]

Comment on attachment 283087 [details] [diff] [review]
Patch 3.0

r=sdwilsh

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

I've updated http://litmus.mozilla.org/show_test.cgi?id=4553 to cover the string change.

in-litmus+

Comment 19

[Ed Lee :Mardak]

I don't think this needs a ui-r because it does what the current design in bug 397655 says to do. (Minus actually removing the whole download from the db and not just from the list.. but that's a backend change.)

Comment 20

[Shawn Wilsher :sdwilsh]

Requesting wanted-firefox3 since this is blocking a wanted bug.  Still need ui review...

Comment 21

[u88484]

Comment on attachment 283087 [details] [diff] [review]
Patch 3.0

I can't follow the requirements for needing approval and such since it seems to change weekly, so requesting approval.

Comment 22

[Reed Loden [:reed]]

Checking in toolkit/locales/en-US/chrome/mozapps/downloads/downloads.dtd;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.dtd,v  <--  downloads.dtd
new revision: 1.16; previous revision: 1.15
done
Checking in toolkit/mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v  <--  downloads.js
new revision: 1.102; previous revision: 1.101
done
Checking in toolkit/mozapps/downloads/content/downloads.xul;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.xul,v  <--  downloads.xul
new revision: 1.33; previous revision: 1.32
done

Comment 23

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b2pre) Gecko/2007111404 Minefield/3.0b2pre