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)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9alpha8

People

(Reporter: ispiked, Assigned: Mardak)

References

Details

Attachments

(2 files, 6 obsolete files)

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".
Attached patch "Less than XX seconds" v1 (obsolete) — Splinter Review
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
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #273803 - Flags: review?(sdwilsh)
Need to hear from beltzner/mconnor before I review
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
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)
Attachment #274101 - Flags: ui-review?(beltzner)
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-
(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 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+
(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 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+
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)
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 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+
(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.
Attached patch unbitrot (obsolete) — Splinter Review
Updated for Bug 388517 and changed NaN to Infinity, so no comment needed.
Attachment #274734 - Attachment is obsolete: true
Attached patch unbitrotSplinter Review
like i said.. no comment necessary...
Attachment #276333 - Attachment is obsolete: true
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
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: