Closed
Bug 844606
Opened 12 years ago
Closed 11 years ago
Clicking "Remove From History" in the downloads view removes only a single entry when several items are selected
Categories
(Firefox :: Downloads Panel, defect)
Tracking
()
VERIFIED
FIXED
Firefox 24
People
(Reporter: andriys, Assigned: Optimizer)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file, 3 obsolete files)
2.84 KB,
patch
|
Optimizer
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0 Build ID: 20130220104816 Steps to reproduce: 1. Click "Downloads" from the Firefox menu. 2. In the Library window select several items from the Downloads list (using either Shift+Click or Ctrl+Click). 3. Right-Click on a selected item and choose "Remove From History" in the context menu. Actual results: Only the topmost selected item in the list gets removed. Expected results: All selected items in the list are removed.
![]() |
||
Updated•12 years ago
|
Component: Untriaged → Bookmarks & History
Updated•12 years ago
|
Status: UNCONFIRMED → NEW
Component: Bookmarks & History → Downloads Panel
Ever confirmed: true
Comment 2•12 years ago
|
||
We would have tracked it as a blocker in case. It's too late for this kind of changes in 20.
Another report: http://forums.mozillazine.org/viewtopic.php?f=9&t=2686847
Additional notes on bug: Introduced in FF20, still present in 20.1 If multiple entries are selected from downloads list only the first item selected is deleted. However if multiple items (in order) are selected from the bottom of the downloads list all the selected items are deleted.
Updated•12 years ago
|
Summary: Clicking "Remove From History" in the download panel removes only a single entry when several items are selected → Clicking "Remove From History" in the downloads view removes only a single entry when several items are selected
Comment 6•12 years ago
|
||
When using the old UI by setting browser.download.useToolkitUI to true the delete behaves correctly. The bug is only present on some download lists. Doing a clear download solve it at least temporarily.
Assignee | ||
Comment 7•12 years ago
|
||
This patch: 1) While removing a single item, deselects the item first and then removes the item. 2) Selects the next entry only when there is one item in the selectedItems. 3) Cache the length of selectedItems and remove the first item, length number of times from the selectedItems. (Since the length of selectedItems will never be zero) I switched to while from for .. of loop as for .. of loop caches the index to run on next. So when i=0, and I deselect the item i=0, the current i=1 becomes i=0 which is skipped by the for of loop as it runs on i=1 next.
Assignee | ||
Comment 8•12 years ago
|
||
matching the code as per the comment. Now when the selection is at the end of the list, selects the previous sibling.
Attachment #744110 -
Attachment is obsolete: true
Attachment #744110 -
Flags: review?(mak77)
Attachment #744111 -
Flags: review?(mak77)
Assignee | ||
Updated•12 years ago
|
Attachment #744111 -
Flags: review?(mak77) → review?(mconley)
Comment 9•12 years ago
|
||
Comment on attachment 744111 [details] [diff] [review] patch v0.2 Redirecting to Mano, who knows more about this code.
Attachment #744111 -
Flags: review?(mconley) → review?(mano)
Comment 10•12 years ago
|
||
Wow, so, listbox.selectItems is just a plan js field. :-/ I prefer cloning the array (through Array.slice) in this case.
Comment 11•12 years ago
|
||
Comment on attachment 744111 [details] [diff] [review] patch v0.2 The other change seems fine.
Attachment #744111 -
Flags: review?(mano) → feedback+
Assignee | ||
Comment 12•12 years ago
|
||
(In reply to Mano from comment #10) > Wow, so, listbox.selectItems is just a plan js field. :-/ > > I prefer cloning the array (through Array.slice) in this case. Wouldn't that be really heavy in case when then number of items is huge, lets say 100 to 500 ? Already iterating through each item is heavy and removing 10 items form the list takes a noticeable time (around 1 second). May be the whole deleting and adding mechanism could be refactored, but that would be out of scope for this bug.
Assignee | ||
Comment 13•12 years ago
|
||
Using Array.slice().
Attachment #744111 -
Attachment is obsolete: true
Attachment #745898 -
Flags: review?(mano)
Assignee | ||
Comment 14•12 years ago
|
||
Comment on attachment 745898 [details] [diff] [review] patch v0.3 really want it to not miss this train.
Attachment #745898 -
Flags: review?(mak77)
Comment 15•12 years ago
|
||
Sadly the problem is still present in FF21. So I guess it missed that train. ... and I'd like to extend a bit the scope of the problem: pressing the "delete" button on the keyboard doesn't work also - despite the fact that there are multiple items selected only the first one is removed. ... and even worse - it deselects the rest of the previously selected items. ... and even more worse is the fact that pressing "delete" loses the focus (the next item is not selected) every other press. What I mean is that pressing delete once removes the selected single item and selects the next one which makes it easy to delete the next one too but sadly this only works for the first deletion. The second one gets deleted and no item in the list gets selected making it impossible to delete another item until the user explicitly selects one with the mouse or the keyboard. I really hope this was not intentional and will be fixed very quickly
Assignee | ||
Comment 17•12 years ago
|
||
Hi Eddie and Dimitar, no need to confirm. The fact that this bug is still open, means that it is not even solved on the latest Nightly builds. Once this bug is fixed in Nightly, we will try to get the fix ported to Aurora (and / or Beta) Channels, which means that it can only land as latest as Firefox 22 in the best case scenario.
Updated•11 years ago
|
Attachment #745898 -
Flags: review?(mak77)
Comment 18•11 years ago
|
||
Comment on attachment 745898 [details] [diff] [review] patch v0.3 Please add a comment explaining that the the reason for cloning the array (just that selectedItems is updated lively and that doCommand may alter the selection).
Attachment #745898 -
Flags: review?(mano) → review+
Assignee | ||
Comment 19•11 years ago
|
||
Carry forward r+. Added the comment explaining everything.
Attachment #745898 -
Attachment is obsolete: true
Attachment #756391 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 20•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/311949ad294f
Keywords: checkin-needed
Comment 21•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/311949ad294f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Comment 23•11 years ago
|
||
Comment on attachment 756391 [details] [diff] [review] patch v0.4 The fix looks safe enough for uplifting and the ui behavior is improved (this was one of the top feedback from input), so I think it's ok for Aurora. On the other side, this is not critical enough to do an evaluation for Beta imo. [Approval Request Comment] Bug caused by (feature/regressing bug #): downloads panel feature User impact if declined: multiple selection in the Library misbehaves Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none
Attachment #756391 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #756391 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 24•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/536e5dd30998
status-firefox23:
--- → fixed
status-firefox24:
--- → fixed
Comment 26•11 years ago
|
||
There's something strange about this bug : it appeared when the download manager was unified with the history manager (a thing that I still don't understand as the two tools haven't the same behaviour at all and shouldn't be linked) but if you select many items in history and delete them at once it works, and it doesn't work only for the downloads.
Comment 27•11 years ago
|
||
(In reply to tech from comment #26) > There's something strange about this bug : it appeared when the download > manager was unified with the history manager (a thing that I still don't > understand as the two tools haven't the same behaviour at all and shouldn't > be linked) but if you select many items in history and delete them at once > it works, and it doesn't work only for the downloads. nothing strange, those views use different code.
Comment 28•11 years ago
|
||
Tested with 22.0 (release channel) and the issue remains UNFIXED, boys and girls. Let's not be too quick to dismiss active bugs, eh?
Comment 29•11 years ago
|
||
(In reply to Marty Schrader from comment #28) > Tested with 22.0 (release channel) and the issue remains UNFIXED, boys and > girls. Let's not be too quick to dismiss active bugs, eh? as indicated in the target milestone and tracking flags this bug is fixed from Firefox 23 on.
Comment 30•11 years ago
|
||
(In reply to Marty Schrader from comment #28) > Tested with 22.0 (release channel) and the issue remains UNFIXED, boys and > girls. Let's not be too quick to dismiss active bugs, eh? It's fixed in FF23+, read the flags.
Comment 31•11 years ago
|
||
Ah, so one has to be subscribed to the devel build channel to see these fixes? Sorry about that, all. Will be more careful next time.
Comment 32•11 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #24) > https://hg.mozilla.org/releases/mozilla-aurora/rev/536e5dd30998 Hello, as I don't want to wait for the FF 23 release dealing with this boring bug, I would like to correct the file by myself before, but I don't find the file allDownloadsViewOverlay.js anywhere in my system. Could someone indicate me where it is located ?
Comment 33•11 years ago
|
||
(In reply to tech from comment #32) > (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #24) > > https://hg.mozilla.org/releases/mozilla-aurora/rev/536e5dd30998 > > Hello, as I don't want to wait for the FF 23 release dealing with this > boring bug, I would like to correct the file by myself before, but I don't > find the file allDownloadsViewOverlay.js anywhere in my system. Could > someone indicate me where it is located ? Instead of monkey-patching your release version of Fireox, I'd suggest just getting a copy of Firefox 23 (Beta) here - http://www.mozilla.org/en-US/firefox/beta/ You don't need to uninstall your release version of Firefox either - you can run either (but not simultaneously, unless you're using separate profiles). And then once Firefox 23 is released, you can remove the beta, update your release version, and you should be good to go. Or you could stay on Beta - we love Beta testers. :)
Comment 35•11 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0 I can't reproduce this issue at all on builds with the bug, so I can't verify it either. Can someone who reproduces it verify it on Firefox 23 and 24 please?
Keywords: verifyme
Assignee | ||
Comment 36•11 years ago
|
||
(In reply to Ioana Budnar, QA [:ioana] from comment #35) > Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0 > > I can't reproduce this issue at all on builds with the bug, so I can't > verify it either. > > Can someone who reproduces it verify it on Firefox 23 and 24 please? Are you saying that using Firefox 22, when you select multiple entries in the downloads view and either press Delete, or right click and choose "Remove from history", All the selected entries get removed ???
Comment 37•11 years ago
|
||
No, I see the bug in FF22 (and it's normal because it's fixed in FF23+). If I select 3 entries in the middle of the download list, only 1 is removed. See screenshot with 3 entries selected, only 1 is deleted: http://i.imgur.com/yf3bzQA.jpg
Comment 38•11 years ago
|
||
(In reply to Girish Sharma [:Optimizer] from comment #36) > Are you saying that using Firefox 22, when you select multiple entries in > the downloads view and either press Delete, or right click and choose > "Remove from history", All the selected entries get removed ??? Yes. I selected multiple entries (Ctrl+Click/Shift+Click), then I right-clicked one of them and chose "Remove from history". All the selected entries were removed. I tried this at least 10 times with different downloads, selecting different amounts of entries, selecting them with Shift some times and Ctrl other times. It worked every time.
Comment 41•11 years ago
|
||
(In reply to Girish Sharma [:Optimizer] from comment #40) > I meant I can this bug present in Fx 22. Can you please verify it's fixed on Firefox 23 and 24 then? :) ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/23.0b9-candidates/build1/ ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/
Assignee | ||
Comment 42•11 years ago
|
||
I downloaded both Firefox beta and aurora and this issue it not visible there, while it is visible in Firefox 22. But I have to ask. I fixed this issue, am I the right person to verify ?
Comment 43•11 years ago
|
||
Andriy, since you reported this issues can you please verify it's fixed? It should be fixed in the following builds: ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/23.0b9-candidates/build1/ ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-aurora/ Thanks
Flags: needinfo?(andriys)
Whiteboard: [qa-]
Reporter | ||
Comment 44•11 years ago
|
||
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #43) > Andriy, since you reported this issues can you please verify it's fixed? It > should be fixed in the following builds: The issue is fixed in both latest FF23 beta and aurora.
Flags: needinfo?(andriys)
You need to log in
before you can comment on or make changes to this bug.
Description
•