User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; es-ES; rv:1.9a8pre) Gecko/2007082904 Minefield/3.0a8pre Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; es-ES; rv:1.9a8pre) Gecko/2007082904 Minefield/3.0a8pre when pausing the download in DM, searching for an item, and clearing the text field, will remove the download status text below the statusbar. Reproducible: Always Steps to Reproduce: 1. Install latest minefield 2. Find a site to download something. (eg. getfirefox.com) 3. Pause download on the DM, and notice the status says "paused - x.x of x.x MB" 4. in the search field, type in something, and then clear the text field immediately 5. Verify the pause status below the status bar is now blank. See screenshot Actual Results: Download status is gone after clearing search field Expected Results: Download status should remain in the DM.

The "pause" step is critical; I tried this without pausing it, and it works fine. (Just a datapoint.)

http://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=4602 in-litmus+

-> sdwilsh until further notice

I'm in the process of storing currBytes and maxBytes into the DB on state changes which can be used to generate the status message for paused downloads. Additionally, this could be used to display the "<size> in <time>" status for "Done" downloads (bug 223895). DownloadProgressListener would need to provide a "generateStatus" function (DPL already has the related strings from the stringbundle).. or we can just keep it in downloads.js and reload the stringbundle there. Also, createDownloadItem would need to additionally take in an end time, curr bytes, max bytes. (sdwilsh: why not just create a nsDownload instead of duplicating all these properties in the richlistitem node? slow? memory?)

(In reply to comment #5) > DownloadProgressListener would need to provide a "generateStatus" function (DPL > already has the related strings from the stringbundle).. or we can just keep it > in downloads.js and reload the stringbundle there. Just have a global var for the stringbundle. > Also, createDownloadItem would need to additionally take in an end time, curr > bytes, max bytes. (sdwilsh: why not just create a nsDownload instead of > duplicating all these properties in the richlistitem node? slow? memory?) Slow - that would do one SELECT per download, which is slower than doing one select and iterating over the rows.

Bug 394548 will have the backend changes needed to help fix this display issue.

Patch notes.. - I've got 4 other patches applied above this one in mq including the cleanup for bug 385734. - Lots of movement from DownloadProgressListener.js to downloads.js... - (lastSeconds shouldn't have been part of the global DPL to begin with :p) - Nothing special needed for pause state (doesn't fix bug 394546, but interestingly enough, now it'll keep updating as Paused -- X of X instead of switching to time left -- X of X :p) - OnDownloadStateChange calls updateStatus after updating state - createDownload doesn't take a status, but takes 2 sizes - gStr holds string names that get converted to strings ;) - gQuery to share the common query parts - updateStatus is mostly like the old code, but updated to grab data from attrs and check for null aDownload - update queries to use gQuery

We're using javascript 1.8 for chrome js, so let's take advantage! :) + let getStr = function(string) sb.getString(string); function expression closure + for (let [name, value] in Iterator(gStr)) a 1.7 idea of destructured assignment, but in 1.8, you can't do that with plain objects because it confuses things with arrays, so explicitly use Iterator around the object -- fixed in bug 366941

The patch depends on bug 385734.

Fix comment *s and gQuery -> gBaseQuery.

Comment on attachment 279527 [details] [diff] [review] v1.2 r=sdwilsh This patch is a bit messy, but I think this actually cleans things up a lot.

Do SQL statement spacing similar to that of dlmgr c++ code that dmose added. Code motion of the upper utility functions.

Comment on attachment 280438 [details] [diff] [review] v2 let's not mess up blame

Let's see what the interdiff shows for v1.2 and this 2.1..

Comment on attachment 280707 [details] [diff] [review] v2.1 ehh.. interdiff has some fake diffs because the new utility functions that I added are now in the bottom utility functions group (v1.2 had them in the top group)

Comment on attachment 280707 [details] [diff] [review] v2.1 r=sdwilsh, but I'd like another reviewer to look it over as well.

Comment on attachment 280707 [details] [diff] [review] v2.1 r=mano, though, to be honest, I'm not sure this potential pref is worth the complexity.

(In reply to comment #18) > I'm not sure this potential pref is worth the complexity. What pref are you referring to exactly?

Checking in toolkit/mozapps/downloads/content/DownloadProgressListener.js; /cvsroot/mozilla/toolkit/mozapps/downloads/content/DownloadProgressListener.js,v <-- DownloadProgressListener.js new revision: 1.26; previous revision: 1.25 done Checking in toolkit/mozapps/downloads/content/downloads.js; /cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v <-- downloads.js new revision: 1.91; previous revision: 1.90 done

Verified FIXED with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007101804 Minefield/3.0a9pre