Closed Bug 817006 Opened 7 years ago Closed 7 years ago

"Remove from list" option in the panel is confusing

Categories

(Firefox :: Downloads Panel, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 20

People

(Reporter: mak, Assigned: mconley)

References

Details

Attachments

(1 file, 3 obsolete files)

In the panel we have "Remove from list", that basically means "I've handled this notification and don't care to see it anymore".

Unfortunately what the user interprets there is "Remove this download from my browser downloads list", that is not what happens, this is a notification panel, not a manager, so the download STAYS in the Library downloads view.

2 possibilities I see:

1. change the label to "Remove notification"

2. since the panel is reduced to just the last N items, don't provide any "remove" option, if the user wants to get rid of it will do from the manager
Flags: needinfo?(shorlander)
Gentle UX-ping, we are still waiting for an interaction analysis.
Answer in this etherpad I think : https://firefox-ux.etherpad.mozilla.org/remaining-download-panel-ux

"Remove from list" option in the panel is confusing

    https://bugzilla.mozilla.org/show_bug.cgi?id=817006

    Remove from List = Remove from History also"
Assignee: nobody → mconley
So, here's one solution - we change the label to make it clearer to the user that removing the item removes the download from the downloads history.
Another solution is to keep "Remove from List", but to simply not have the download be removed from the history.

I'm investigating this solution - but my instinct is that we might expose bugs if these two lists get out of sync...
Notice the atm we do NOT remove the download from history...
The other solution would be "Dismiss this notification", but I think we want this to also remove the download from history, so your first solution is fine provided you also fix the functionality.
Attached patch Patch v1 (obsolete) — Splinter Review
Switches the string from "Remove from List" to "Remove from History", and updates DownloadDataItem's remove method to also remove itself from the browsing history.
Attachment #691851 - Attachment is obsolete: true
Comment on attachment 691892 [details] [diff] [review]
Patch v1

The relationship between the downloads database and the browsing history is a little unclear to me...but this patch appears to do the job.

Removing a download from the panel now removes it from the Places view.
Attachment #691892 - Flags: review?(mak77)
Talked to Mike and settled on the current approach.
Flags: needinfo?(shorlander)
Attached patch Patch v2 (obsolete) — Splinter Review
As discussed in IRC, we're opting to remove the download from the browsing history in downloads.js (as opposed to within DownloadDataItem's remove method).
Attachment #691892 - Attachment is obsolete: true
Attachment #691892 - Flags: review?(mak77)
Attachment #691902 - Flags: review?(mak77)
Comment on attachment 691902 [details] [diff] [review]
Patch v2

Review of attachment 691902 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/downloads/content/downloads.js
@@ +48,5 @@
>                                    "resource://gre/modules/PrivateBrowsingUtils.jsm");
> +XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
> +                                  "resource://gre/modules/PlacesUtils.jsm");
> +XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
> +                                  "resource://gre/modules/NetUtil.jsm");

since you are importing this, in _onKeyDown there is a Services.io.newURI(url, null, null); that could be replaced
Attachment #691902 - Flags: review?(mak77) → review+
Fixed! Thanks Marco!
Attachment #691902 - Attachment is obsolete: true
Unfortunately, I had to back this out when I was backing out the patch for bug 675902, because this one seems to depend on that one (or at least I got merge conflicts from Mercurial).  Backout changeset: https://hg.mozilla.org/integration/mozilla-inbound/rev/f2d9eaf13c19
Serves me right for landing something on inbound that relied on something that hadn't been merged into central yet. Thanks for your work.
https://hg.mozilla.org/mozilla-central/rev/048673b6b48b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Verified as fixed on the latest Nighlty - "Remove from list" option in the panel is now replaced with "Remove From History" (and after selecting the option to "Remove from History" the downloaded item is removed from the Panel and from Library -> Downloads). 


Verified on Windows 7, Ubuntu 12.10 and Mac OS X 10.7.5:
Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20121217 Firefox/20.0 Build ID: 20121217030850
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121218 Firefox/20.0 Build ID: 20121218030803
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:20.0) Gecko/20121218 Firefox/20.0 Build ID: 20121218030803
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.