381535 - Jesse's downloads.rdf doesn't import to sqlite

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Jesse Ruderman]

Attachment: Jesse's downloads.rdf

This downloads.rdf has a single entry, Google.htm.  This morning's Firefox nightly created it, or at least added to it.

When I have this downloads.rdf in my profile and launch an hourly, the entry doesn't appear in the download manager or in downloads.sqlite.

Comment 1

[Shawn Wilsher :sdwilsh]

>    <NC:DateEnded NC:parseType="Date">Mon May 21 21:32:50 PDT 2007 +368521</NC:DateEnded>

No NC:DateStarted, which is somewhat important.  The question is, do we really want to fail this way?  That time is somewhat important, but I suppose if either one fails we could use the end time.

But then, that introduces a lot of code bloat to our downloads history, and do we really want that?  Is it important enough?

Comment 2

[Jesse Ruderman]

If I use this morning's nightly and alt+click a link to http://www.google.com/, I get a downloads.rdf entry without a DateStarted.  So I think you should at least deal with a missing DateStarted.

Comment 3

[Mike Connor [:mconnor]]

we should just make the beginning and end dates match, if this is something that might happen.  Tossing the entry entirely seems unnecessary.

Comment 4

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

This wasn't hard to fix at all. Complete with a test case too!

Comment 5

[Mike Connor [:mconnor]]

Comment on attachment 265885 [details] [diff] [review]
v1.0

r=me, though is DateEnded guaranteed to be there? and if its not, what does that impact?  Can't we just fill in a random old date?

Comment 6

[Shawn Wilsher :sdwilsh]

I don't see why it *wouldn't* be there, but at the same time, I thought the same about DateStarted.  If it isn't there, we just won't import that download from the history.  I'm not sure I like the idea of putting in some random date, since that will change the order of the downloads.

Comment 7

[Shawn Wilsher :sdwilsh]

If the DateEnded being gone is an issue, I'd like to open up a new bug for that.

Checking in toolkit/components/downloads/src/nsDownloadManager.cpp;
new revision: 1.80; previous revision: 1.79
Checking in toolkit/components/downloads/test/unit/head_download_manager.js;
new revision: 1.2; previous revision: 1.1
Checking in toolkit/components/downloads/test/unit/test_download_manager_migration.js;
new revision: 1.4; previous revision: 1.3
Checking in toolkit/components/downloads/test/unit/test_bug_381535.js;
initial revision: 1.1
Checking in toolkit/components/downloads/test/unit/bug_381535_downloads.rdf;
initial revision: 1.1

Comment 8

[Jesse Ruderman]

Verified fixed. My original downloads.rdf (which had this problem *and* a completely corrupted entry) now imports as expected.