73685 - icons disappear in threadpane outliner

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P2)

Description

[(not reading, please use [e-mail] instead)]

the mailnews threadpane now has icons.

I've noticed that icons aren't always showing up, especially when I select a row.

if I force a repaint, they show up.

hyatt tells me this is a libpr0n problem.

assigning to pavlov

Comment 1

[Stuart Parmenter]

Attachment: patch to maybe fix problem

Comment 2

[Ninoschka Baca]

Build 2001-03-28-04: NT4, Mac 9.04
Two things that I've noticed.

1.When I first open mail the green star/grey circle icon does not appear. After 
scrolling then they appear.

2. In the flag area a grey circle displays, select the flag area and the grey 
circle disappears, select the same area again and the grey circle reappears, 
select the area a third time and now the flag icon appears. This only happens 
the first time I try to set a flag in a session. Afterwards I have no problems 
selecting other messages for a flag to appear.

Comment 3

[Stuart Parmenter]

Attachment: better patch

Comment 4

[Stuart Parmenter]

hyatt: want to sr= this ?

Comment 5

[(not reading, please use [e-mail] instead)]

*** Bug 74126 has been marked as a duplicate of this bug. ***

Comment 6

[Mike Shaver (:shaver emeritus)]

I don't like the duplicated GetRect/OnDataAvailable code there: can you factor
the common (identical) code out to before the if?

Comment 7

[Stuart Parmenter]

The duplicated GetRect/OnDataAvailable is done that way on purpose.  One of 
them is right and one if them isn't.  See the comment next to them.  If the 
image hasn't fully loaded, we send the full rect, which isn't correct as we 
should send a smaller rect, but this will simply cause us to draw a slightly 
larger rect.

Comment 8

[Brendan Eich [:brendan]]

pav: what would the smaller rect's dimensions be?  How come you can't send it
now?  The comment doesn't say enough.

/be

Comment 9

[Stuart Parmenter]

they would be whatever part of the image has been decoded so far.  there arn't 
any APIs to get this info from right now, and some serious thought needs to go 
into where the apis should go before i put them there.

Comment 10

[Brendan Eich [:brendan]]

Then please to be filing a new bug and marking a dependency, or at least
recording the number here.

Get a non-rubberstamp r= from saari, I'll sr=.

/be

Comment 11

[Stuart Parmenter]

filed bug 75182

Comment 12

[Stuart Parmenter]

moving to browser

Comment 13

[Stuart Parmenter]

imagelib

Comment 14

[Stuart Parmenter]

Attachment: better fix.. removes hack code from outliner

Comment 15

[David Hyatt]

sr=hyatt

Comment 16

[Peter "jag" Annema]

Pavlov said he fixed the PR_ASSERT to be on |ob| and not |observer|.

r=jag

Comment 17

[Stuart Parmenter]

fix checked in

Comment 18

[Brendan Eich [:brendan]]

Come on, let's review more carefully.

1.  Use NS_ASSERTION, not PR_ASSERT.

2.  What is wrong with this picture:

-  if (NS_FAILED(status))
+  if (!(mStatus & imgIRequest::STATUS_ERROR) && NS_FAILED(aStatus))
     mStatus |= imgIRequest::STATUS_ERROR;
 
   PRInt32 i = -1;
   PRInt32 count = mObservers.Count();
 
+  nsresult status;
+  if (mStatus & imgIRequest::STATUS_LOAD_COMPLETE)
+    status = NS_IMAGELIB_SUCCESS_LOAD_FINISHED;
+  else if (mStatus & imgIRequest::STATUS_ERROR)
+    status = NS_IMAGELIB_ERROR_FAILURE;
+

2a. No reason to test !(mStatus & imgIRequest::STATUS_ERROR) before
NS_FAILED(aStatus) if you're just going to set the former flag based on the
latter condition.

2b. The i and count variables should be initialized nearer to their uses.

2c.  The final if-else-if manages to mask any error flagged in mStatus, because
it tests for STATUS_LOAD_COMPLETE and reflects that in the rv.  Is that state
bit really exclusive of errors?  It seems not, given the earlier code that sets
the error flag based on aStatus.

/be

Comment 19

[Peter "jag" Annema]

I messed up here.

I actually thought about the PR_ASSERT and ass-u-me-d it was a Pav-ism /
libpr0n-ism...

I thought about 2a and figured it was trading a read for an unnecessary write.
Silly thoughts, in hind-sight. Clarity of code by simply OR-assigning prevails
over any gain this could give.

2b I missed, too much of a hurry I guess... I generally point those out myself.

2c ... see 2b...

Nice wake-up call :-/

Comment 20

[Stuart Parmenter]

we've moved nitpicky comments to bug 6074.  marking this one fixed (again) 
since the bug in question no longer exists.

Comment 21

[Ninoschka Baca]

Build 2001-04-17-04: NT4, Mac 9.04 - Fixed

Build 2001-04-17-08: Linux RH 6.2
Reopening since it does not appear fixed on linux.

1. Read column: When mail first opens the green stars for messages that are 
Unread display, which is good but the grey circles that should appear for 
messages read do Not appear. After I force a redraw then they appear.

2. Flag column: The same is true for the flag column. If I don't force a redraw 
then the same problem occurs as previously reported on 3/28/2001.

Comment 22

[Stuart Parmenter]

please file a new bug.  this one is fixed.  the drawing problem on linux could 
probably be duped onto about 50 other bugs.

Comment 23

[Stuart Parmenter]

remarking fixed.

Comment 24

[Ninoschka Baca]

Verified Fixed. 
Created a new bug for initial paint problem (bug# 76690).