With bug 394516 fixed, there's a DownloadsUtils javascript module that knows how to display download progress for size and time and other fun stuff. http://mxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/src/DownloadUtils.jsm#39

I think the update code still downloads chunks of the file when in the foreground, rather than just retrieving the file in one go. The constants are defined at http://mxr.mozilla.org/seamonkey/source/toolkit/mozapps/update/src/nsUpdateService.js.in#108

I took a quick look and it seems like all of DownloadStatusFormatter could be replaced. http://mxr.mozilla.org/seamonkey/source/toolkit/mozapps/update/content/updates.js#897 onProgress has (progress, maxProgress) as parameters and there's some existing code that computes the elapsed time to estimate speed.

Note - this has to handle pausing and unpausing too, so not sure if you can completely remove that chunk of code.

Bye bye DownloadStatusFormatter. Hrm.. should I r? mconnor or someone else?

Btw, you can set app.update.url.override to point to a file like attachment 252167 [details] and do check updates.

You can use one of these builds and set app.update.url.override to see the changes. https://build.mozilla.org/tryserver-builds/2008-03-04_18:08-edward.lee@engineering.uiuc.edu-download/

Comment on attachment 307308 [details] [diff] [review] v1 Note: I'm not a peer of this code, so my review doesn't necessarily mean much. > /** >+ * Update download progress status >+ */ >+ _updateDownloadStatus: function(aCurr, aMax) { nit: javadoc comment should define aCurr and aMax r=sdwilsh

late-l10n because we are removing strings.

(In reply to comment #7) > r=sdwilsh > Note: I'm not a peer of this code, so my review doesn't necessarily mean much. mconnor last reviewed code that touched toolkit/mozapps/update/content/updates.js ;) > > /** > >+ * Update download progress status > >+ */ > >+ _updateDownloadStatus: function(aCurr, aMax) { > nit: javadoc comment should define aCurr and aMax Oops. That's what I get for not finishing my comments ;) Added @params

Moved the import to outside so that it doesn't try importing each time it's called and noticing that it already imported.

If we're really attempting to not make string changes, you should leave the unused strings and file a followup to remove them after we branch/ship.

does removing strings really matter? I can see changing a string, but just removing it seems like a trivial thing to do (but then, I'm not a localizer)

Bug 405720 comment 13: Note, removing unused strings is fine, as that's not breaking localizations.

Removing them is trivial, but it does require attention from the localizer. It does "break" locales in that it turns tinderbox orange (though the actual builds should still work OK). I'm not sure how buildbot handles them.

String removals aren't late-l10n...

nominating for wanted...

I think it's about time to even cut down on string removals. This should be treated as low-risk late-l10n, IMHO.

Does this have an impact on help content? Like, would screenshots change, or something?

No, it should look exactly the same. It's just removing duplicated code (this display code was duplicated in three places in the codebase). Mind you, if locales were not consistent across the three places it could impact screenshots.

Actually, the display will be slightly different because we display "5 seconds" instead of "00:05" and various other changes of how we display progress in the download manager.

This will not block the final release of Firefox 3. Any patch will need unit tests in order to be approved.

I like the idea of app update using DownloadUtils but the progress strings wrap under some circumstances in the ui.

Would things fit if we moved the Pause button to somewhere else? Or we could split up the text. DownloadUtils provides individual strings for transfer total (5.1 of 10.2 MB) as well as download times (3 minutes remaining) But nothing for rate by itself.. If we don't care about the rate (no string changes), it could look like.. .~, 3 minutes remaining [Pause] `-' 5.1 of 10.2 MB ^^ that's the activity spinner thing

I'll try to think of some other possible layouts. I recall the wizard page being the incorrect size on Windows as wekk so it might fit with the correct size

This shouldn't be taken care of in this bug (if at all) but the width for the wizard can definitely be increased on windows. Also iirc, wizard widths and heights are fixed sized for each locale on windows.

Edward, I'll approve this after beta 1 and I'll take on the UI changes as part of bug 324121.

Comment on attachment 307555 [details] [diff] [review] v1.2 Edward, thanks and please land this after the tree reopens after beta 1. I'm working on changes to the ui to accommodate the additional space required in bug 324121