Closed Bug 954675 Opened 10 years ago Closed 10 years ago

Only show progress bar when necessary

Categories

(Instantbird Graveyard :: Conversation, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

Attachments

(1 file, 1 obsolete file)

*** Original post on bio 1243 at 2012-01-24 21:13:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1243 as attmnt 1131 at 2012-01-24 21:13:00 UTC ***

Only show progress bar if after the third call to display pending messages (ca 120ms) less than half have already been displayed.
Attachment #8352876 - Flags: review?(florian)
*** Original post on bio 1243 at 2012-01-25 10:39:40 UTC ***

Comment on attachment 8352876 [details] [diff] [review] (bio-attmnt 1131)
Patch

The patch looks good (I haven't actually tested it though).

>@@ -325,8 +322,13 @@
>             }
>             if (i < max - 1) {
>               this._nextPendingMessageIndex = i + 1;
>-              if (this.progressBar)
>+              if (this.progressBar) {

I think the comment you wrote in the bug should also be here ;). The "2" constants look a bit surprising without a comment explaining what they mean.

>+                if (++this._displayPendingMessagesCalls > 2 &&
>+                    max/this._nextPendingMessageIndex > 2)

Nit: space before and after the / operator.
Or you can change the line to max > 2 * this._nextPendingMessageIndex to only deal with integer values.
Attached patch PatchSplinter Review
*** Original post on bio 1243 as attmnt 1137 at 2012-01-25 19:10:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352882 - Flags: review?(florian)
Comment on attachment 8352876 [details] [diff] [review]
Patch

*** Original change on bio 1243 attmnt 1131 at 2012-01-25 19:10:12 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352876 - Attachment is obsolete: true
Attachment #8352876 - Flags: review?(florian)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment on attachment 8352882 [details] [diff] [review]
Patch

*** Original change on bio 1243 attmnt 1137 at 2012-01-25 23:01:43 UTC ***

This looks good. The only thing I see that would need to be adjusted is that _displayPendingMessagesCalls should probably be reset in the onStateChange method. I'll handle that before checking this in.
Attachment #8352882 - Flags: review?(florian) → review+
*** Original post on bio 1243 at 2012-01-26 00:43:15 UTC ***

Checked in as http://hg.instantbird.org/instantbird/rev/91475f830792

Thanks for fixing this. :)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.