icons disappear in threadpane outliner

VERIFIED FIXED in mozilla0.9

Status

()

P2
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: sspitzer, Assigned: pavlov)

Tracking

Trunk
mozilla0.9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

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
(Assignee)

Comment 1

18 years ago
Created attachment 29098 [details] [diff] [review]
patch to maybe fix problem
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Keywords: patch
OS: Windows NT → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla0.9

Comment 2

18 years ago
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.
(Assignee)

Comment 3

18 years ago
Created attachment 29657 [details] [diff] [review]
better patch
(Assignee)

Comment 4

18 years ago
hyatt: want to sr= this ?
*** Bug 74126 has been marked as a duplicate of this bug. ***
I don't like the duplicated GetRect/OnDataAvailable code there: can you factor
the common (identical) code out to before the if?
(Assignee)

Comment 7

18 years ago
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.
pav: what would the smaller rect's dimensions be?  How come you can't send it
now?  The comment doesn't say enough.

/be
(Assignee)

Comment 9

18 years ago
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.
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
(Assignee)

Comment 11

18 years ago
filed bug 75182
(Assignee)

Comment 12

18 years ago
moving to browser
Component: Mail Window Front End → Internationalization
Product: MailNews → Browser
(Assignee)

Comment 13

18 years ago
imagelib
Component: Internationalization → ImageLib
(Assignee)

Comment 14

18 years ago
Created attachment 30098 [details] [diff] [review]
better fix..  removes hack code from outliner

Comment 15

18 years ago
sr=hyatt

Comment 16

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

r=jag
(Assignee)

Comment 17

18 years ago
fix checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 19

18 years ago
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 :-/
(Assignee)

Updated

18 years ago
Status: REOPENED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 20

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

Updated

18 years ago
QA Contact: esther → nbaca

Comment 21

18 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 22

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

Comment 23

18 years ago
remarking fixed.
Status: REOPENED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 24

18 years ago
Verified Fixed. 
Created a new bug for initial paint problem (bug# 76690).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.