Closed Bug 837117 Opened 7 years ago Closed 7 years ago

Downloaded items change position in the downloads view if Clear List is selected

Categories

(Firefox :: Downloads Panel, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 21
Tracking Status
firefox20 --- verified

People

(Reporter: simona.marcu, Assigned: mak)

References

Details

Attachments

(2 files)

Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130201 Firefox/21.0
Build ID: 20130201030927

Please see the screencast for more details.
http://screencast.com/t/Mvr1d5m7T

Steps to reproduce:
1. Launch Firefox
2. Open the Downloads View: Tools menu -> Downloads
3. Perform 5 downloads of your choice and observe their position in the Downloads View
4. Open the downloads Panel, right click on any of the downloads and select Clear List

Expected results:
The panel's list is cleared and the position of the downloads in the Downloads View is the same.

Actual results:
The panel is cleared but some of the downloads from the Downloads view change position.
Also, after the Clear List action some of the downloads have a different state: some of them have an Unknown size and others are failed. (please see the screencast before and after the Clear List action).
taking for investigation
Assignee: nobody → mak77
Attached patch patch v1.0Splinter Review
typos ftw!
Attachment #710283 - Flags: review?(mano)
Comment on attachment 710283 [details] [diff] [review]
patch v1.0

[Approval Request Comment]
Bug caused by (feature/regressing bug #): downloads panel feature
User impact if declined: Clearing downloads list causes view corruption
Testing completed (on m-c, etc.): m-i
Risk to taking this patch (and alternatives if risky): limited to the feature, fixes typos
String or UUID changes made by this patch: none
Attachment #710283 - Flags: approval-mozilla-aurora?
Attachment #710283 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/0edd11517bcd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20130211 Firefox/21.0 (20130211031055)
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20130212 Firefox/20.0 (20130212042017)

I can still reproduce this issue on the latest Nightly and Aurora - the position of the downloads is still changed after Clearing the list from the panel. Moreover one of the downloads is deleted from the downloads view after clearing the list.

Steps to reproduce:
1. Launch Firefox
2. Open the downloads view
3. Perform 3 downloads 
4. Navigate to: http://bit.ly/159c6t5 and save the first image (save is as it is "index.jpeg")
5. Perform 3 more downloads.
6. Open the Panel, right click on any download and select Clear List.
7. Close the Library window and open it again.

Actual results:

After step 6:
- The downloads list gets reversed.
- The index.jpeg file is removed from the downloads view.
After step 7:
- The downloads list gets reversed again (the order of the downloads is the same as before step 6).
- The index.jpeg file is still missing from the downloads view.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Thank you Simona, everything was working when I tested it, but I surely missed some edge case here :(
I'm investigating your steps to reproduce now, will make a follow-up patch once I figure what's up.
(In reply to Simona B [QA] from comment #8)
> - The index.jpeg file is removed from the downloads view.

This is expected, cause this image is a data: uri, we don't store data: uris in history, since it is not in history, when it's removed from the downloads list it also disappears from the view.
Here you can find list of protocols we don't add to history:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/nsNavHistory.cpp#1256

Now investigating the order issue.
There is another edge case that I just found, due to a bug in toolkit we never add downloads history for 0-byte files, thus when you clear history they disappear cause we lack their history information.
I filed bug 840676 about that, I don't think we are going to fix it for Aurora considered it's a "minor" issue, but worth to mention it while I found it.
Those entries without any Places data were indeed confusing the code, by causing this._lastSessionDownloadElement to go out-of-sync.  Really good catch with that data uri!

Now doing final testing before asking review.
Comment on attachment 713074 [details] [diff] [review]
part 2 - the missing puzzle piece!

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

verified with Simona's STRs.
Attachment #713074 - Flags: review?(mano)
Comment on attachment 713074 [details] [diff] [review]
part 2 - the missing puzzle piece!

first come first serve!
Attachment #713074 - Flags: review?(mconley)
Comment on attachment 713074 [details] [diff] [review]
part 2 - the missing puzzle piece!

Yep - this makes sense. Good catch!
Attachment #713074 - Flags: review?(mconley) → review+
Attachment #713074 - Flags: review?(mano)
https://hg.mozilla.org/mozilla-central/rev/fdf15fa098d9

Simona, could you please verify this on tomorrow's (13) nightly, so we can uplift to Aurora a verified fix.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Flags: needinfo?(simona.marcu)
Resolution: --- → FIXED
Comment on attachment 713074 [details] [diff] [review]
part 2 - the missing puzzle piece!

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Downloads panel feature
User impact if declined: broken order of downloads after removals
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): minimal, just updating internal status. Limited to the feature.
String or UUID changes made by this patch: none
Attachment #713074 - Flags: approval-mozilla-aurora?
Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130212 Firefox/21.0
Build ID: 20130212182646

Verified as fixed on the hourly tinderbox build - the position of the downloads remains the same after using the Clear List option from the panel.

Verified on Windows 7, Ubuntu 12.04 and Mac OS X 10.7.5.
Flags: needinfo?(simona.marcu)
Attachment #713074 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed on Firefox 20 beta 1 - the position of the downloaded items is not changed when clearing the downloads list.

Verified on Windows 7, Ubuntu 12.04 and Mac OS X 10.8:
Build ID: 20130220104816
Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0
QA Contact: simona.marcu
You need to log in before you can comment on or make changes to this bug.