Closed
Bug 389569
Opened 17 years ago
Closed 17 years ago
"Less than a minute" in Download Manager is silly
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha8
People
(Reporter: ispiked, Assigned: Mardak)
References
Details
Attachments
(2 files, 6 obsolete files)
5.00 KB,
patch
|
Details | Diff | Splinter Review | |
23.27 KB,
image/png
|
Details |
Bug 388506 changed the Download Manager to display "Less than a minute" for downloads with times less than one minute. See attachment 272709 [details]. I see no reason for doing this. It hides useful data from users who want to see it in some attempt to be "cute". IIRC, people worked hard on getting the up-to-the-second time calculation to display correctly, too. I'd like to hear an argument for why this is done other than "it's simple". IMO, most people can understand "30 seconds" just as well as they can "Less than a minute".
Assignee | ||
Comment 1•17 years ago
|
||
Show 2 digit seconds when possible, otherwise minutes. 3 min -> 2 min -> 90 secs -> 80 secs -> ... -> 10 secs 2 minutes left -> Less than 90 seconds
Comment 2•17 years ago
|
||
Need to hear from beltzner/mconnor before I review
Comment 3•17 years ago
|
||
The design was more about getting rid of "0:55 seconds remaining". Yeah, I like the idea of ... 3 minutes left 2 minutes left 60 seconds left 59 seconds left ... 5 seconds left ... A few seconds left Amongst other things, it keeps the feeling of progress and speed up. One concern is that if download time variance causes things to jitter, it can be frustrating. I don't like the idea of seeing: 60 seconds left 59 seconds left ... 57 seconds left 60 seconds left
Assignee | ||
Comment 4•17 years ago
|
||
A combination of suggestions: Using the "fuzz" factor of only showing multiples of 10 seconds, this already gives the impression of an estimate; now it'll also avoid jitter by reusing the previous seconds if the new time estimate is one fuzz factor up. This means there needs to be a change of 20 seconds or more in the estimate to have it switch to a bigger time; going down in time is as usual. beltzner: Is the using the "Less than 10x seconds" okay instead of giving the exact second which might imply absoluteness?
Attachment #273803 -
Attachment is obsolete: true
Attachment #273803 -
Flags: review?(sdwilsh)
Assignee | ||
Updated•17 years ago
|
Attachment #274101 -
Flags: ui-review?(beltzner)
Comment 5•17 years ago
|
||
Comment on attachment 274101 [details] [diff] [review] Less than X0 seconds, less-jitter v1 I think we should try to display second-by-second counts if we can; ispiked mentioned that a lot of work went into being able to get those values, and it does make the download "feel" faster as the text keeps moving. I like your fuzzing approach of reusing the last value if the new is off by a round-ing up error to avoid jitter, though, but maybe reduce the fuzz amount in that check so if the values were "41, 40, 38, 40, 38, 37" the user would see "41, 40, 38, 38, 37". Obviously if the time started trending back up we'd have to revise, but we should wait a few ticks to ensure that it's not just a small download speed jitter and actually a full-on slow down. As for replacing with "less than N seconds", I think we should only do that when there's 3 seconds left, as stated above with "a few seconds left". That might be "cute", but I think it's also a little more human.
Attachment #274101 -
Flags: ui-review?(beltzner) → ui-review-
Assignee | ||
Comment 6•17 years ago
|
||
(In reply to comment #5) > I think we should try to display second-by-second counts if we can; ispiked > mentioned that a lot of work went into being able to get those values, and it > does make the download "feel" faster as the text keeps moving. I believe the work mentioned was related to getting the download speed to be accurate, which would still get updated without fuzzing (same with the amount transferred). But continuously updating does make the download seem faster - there are plenty of counters that display into the .01s. > I like your fuzzing approach of reusing the last value if the new is off by a > round-ing up error to avoid jitter, though, but maybe reduce the fuzz amount in > that check so if the values were "41, 40, 38, 40, 38, 37" the user would see > "41, 40, 38, 38, 37". The fuzzing was mainly for rounding to multiples of 10, so it doesn't quite apply for displaying every second. But the logic has been modified to check for an upswing in time and reuse the old time instead. > Obviously if the time started trending back up we'd have > to revise, but we should wait a few ticks to ensure that it's not just a small > download speed jitter and actually a full-on slow down. The current logic only changes the time if the upswing is large enough (greater than 10).. [Should this be a percentage up-change?] It's not too much of an issue for a full slowdown because then the time would either stay at the same value for an extra 10 seconds and by then it'll start counting down again. > As for replacing with "less than N seconds", I think we should only do that > when there's 3 seconds left, as stated above with "a few seconds left". That > might be "cute", but I think it's also a little more human. Done. beltzner: Should it start showing seconds at two digits? At 99 seconds or 60 seconds?
Attachment #274184 -
Flags: ui-review?(beltzner)
Comment 7•17 years ago
|
||
Comment on attachment 274184 [details] [diff] [review] XX seconds left, A few seconds left, less jitter v1 > transferred). But continuously updating does make the download seem faster - > there are plenty of counters that display into the .01s. Heh, let's not go crazy here ;) > The current logic only changes the time if the upswing is large enough (greater > than 10).. [Should this be a percentage up-change?] It's not too much of an > issue for a full slowdown because then the time would either stay at the same > value for an extra 10 seconds and by then it'll start counting down again. Let's stick with 10s, then. We might want to lower it to, say, 3, since that's about the amount of time in which lack of change is often misinterpreted as "frozen" UI, but let's see how it works with what you have now. > beltzner: Should it start showing seconds at two digits? At 99 seconds or 60 At 60 seconds, please.
Attachment #274184 -
Flags: ui-review?(beltzner) → ui-review+
Assignee | ||
Comment 8•17 years ago
|
||
(In reply to comment #7) > 3, since that's about the amount of time in which lack of change > is often misinterpreted as "frozen" UI Using 10 for now. Just to clarify, the UI will still update with the amount transferred and current download speed twice every second (actually every 400ms I believe...) - it's only the time left that get "frozen". > > Should it start showing seconds at two digits? > At 60 seconds Done.
Attachment #274101 -
Attachment is obsolete: true
Attachment #274184 -
Attachment is obsolete: true
Attachment #274689 -
Flags: review?(sdwilsh)
Comment 9•17 years ago
|
||
Comment on attachment 274689 [details] [diff] [review] XX seconds left, A few seconds left, less jitter v2 yay for approving code that will bitrot me horribly...
Attachment #274689 -
Flags: review?(sdwilsh)
Attachment #274689 -
Flags: review?(mano)
Attachment #274689 -
Flags: review+
Assignee | ||
Comment 10•17 years ago
|
||
Oops, setting lastSeconds to -1 worked when I was fuzzing things, but not quite so anymore. NaN will work because all arithmetic results in more NaN and comparisons result in false.
Attachment #274689 -
Attachment is obsolete: true
Attachment #274734 -
Flags: review?(mano)
Attachment #274689 -
Flags: review?(mano)
Assignee | ||
Comment 11•17 years ago
|
||
I suppose technically initializing lastSeconds to Infinity would work as well. ;) And I suppose more correct in some ways... seconds - Infinity --> -Infinity -Infinity > 0 && -Infinity <= 10 --> false && true
Comment 12•17 years ago
|
||
Comment on attachment 274734 [details] [diff] [review] XX seconds left, A few seconds left, less jitter v2.1 Reasonable... I guess, where are your tests? ;) Please file a bug on removing replaceInsert in favor of the stringbundle api.
Attachment #274734 -
Flags: review?(mano) → review+
Comment 13•17 years ago
|
||
(In reply to comment #12) > Please file a bug on removing replaceInsert in favor of the stringbundle api. I almost had him do that before, then we realized that that would increase the code complexity greatly, which is why we didn't do it.
Assignee | ||
Comment 14•17 years ago
|
||
Updated for Bug 388517 and changed NaN to Infinity, so no comment needed.
Attachment #274734 -
Attachment is obsolete: true
Assignee | ||
Comment 15•17 years ago
|
||
like i said.. no comment necessary...
Attachment #276333 -
Attachment is obsolete: true
Comment 16•17 years ago
|
||
Thanks for the patch! Checking in toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties; new revision: 1.7; previous revision: 1.6 Checking in toolkit/mozapps/downloads/content/DownloadProgressListener.js; new revision: 1.19; previous revision: 1.18
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite-
Flags: in-litmus?
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M8
Verified FIXED with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007082923 Minefield/3.0a8pre
Status: RESOLVED → VERIFIED
http://litmus.mozilla.org/show_test.cgi?id=4604 in-litmus+
Flags: in-litmus? → in-litmus+
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•