283123 - Black background flicker when moving to account central

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Scott MacGregor]

I've been noticing this on the trunk Thunderbird builds.

1) Select a folder in the folder pane
2) Now select a top level account.

Note the thread / message pane area flickers black before the Account Central
box is made visible. 

Try this on a build from the aviary 1.0 branch and notice there is no such black
flicker.

Comment 1

[Scott MacGregor]

FYI, this black flicker is present in the mozilla application suite as well as
Thunderbird. The easiest way to see it is to click back and forth between two
accounts in the folder pane.

It appeared between 11/25/2004 and 01/08/2005

still narrowing down the build date. 

Comment 2

[Scott MacGregor]

I *think* this was caused by Bug #244366. I narrowed the regression window down
to checkins that happend on the 19th of January. Builds on the 20th have this
black flicker problem, builds from the 19th do not.

Again, to see this in the mozilla application suite, start up. Click on a folder
like your inbox. Now click on the account in the folder pane. Click back and
forth and notice the black flicker every time the account central panel loads. 

Going through the checkin logs from the 19th, these view manager changes for
244366 seem like the only likely candidate. Unfortunately, I'm having trouble 
back out the changes because there have been so many other changes to the view
manager and layout which now depend on 244366. 

Comment 3

[Boris Zbarsky [:bzbarsky]]

Hmm... so on Linux I don't see black flicker with my build.  I do see, very
rarely, a flicker of the default background color of the XUL (gray in classic,
bluish in modern), but I see that in a Jan 19 nightly too.  

Scott, could you check whether the patches for bug 281157 and bug 282764
(especially the one-liner in the latter) help here?

If they do not, there are two parts of the patch in bug 244366 that can be
disabled independently to test whether they have an effect. 
https://bugzilla.mozilla.org/attachment.cgi?id=174013 disables one of them, and
https://bugzilla.mozilla.org/attachment.cgi?id=174014 disables the other.  If
you can tell me whether one or the other helps, that would give me something to
go on...

Comment 4

[Scott MacGregor]

Attachment: screen shot of the black flicker

The flicker lasts just long enough for my reflexes to get a screen capture.

Comment 5

[Scott MacGregor]

Shucks no luck on any of these suggestions. I'll go through the change log from
the 19th of January again in case I missed something someone else did too.

I tried:

bug 281157, no change
then I tried the one liner patch in bug 282764, no change
https://bugzilla.mozilla.org/attachment.cgi?id=174013, no change,
https://bugzilla.mozilla.org/attachment.cgi?id=174014, at first I thought this
was it but I actually still see the flicker with this patch too.

I didn't try any of these patches together, each patch had its own test run. 

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yeah, that's pretty noticeable.  I'm definitely not seeing that....

I agree with you that the patch from bug 244366 really does seem most likely. 
If you're still seeing the flicker with both attachment 174013 [details] [diff] [review] and attachment
174014 [details] [diff] [review] applied, though, it's something else -- those two between the two of them
completely back out that patch.

Comment 7

[Martijn Wargers (dead)]

Well, I can see this bug also in a 32005-01-19 thunderbird build.
I can't see the bug in a 2004101107 Mozilla trunk build.
I can see the bug in a 2004101206 Mozilla trunk build.
Looking with bonsai in that range makes me guess this could be a regression from
the fix of bug 238493.

Comment 8

[Martijn Wargers (dead)]

Strange, I just tried Thunderbird builds with the same dates as mentioned in
comment 7, but with those I can't see the bug in either of those builds.
I rechecked my observations in comment 7, and I still think they are correct:
- in the 2004101107 build, I only get to see the default background color 'flash'.
- in the 2004101206 build, I also -with the default background color 'flash'-
get to see the black color 'flash'.

Comment 9

[Scott MacGregor]

cc'ing Roc per Martijin's comments that this regression may have come from Bug
#238493 although I haven't been able to quickly back that out yet since a lot of
other changes have gone in on top of it. 

Comment 10

[Boris Zbarsky [:bzbarsky]]

The easiest way to test an effective backout of bug 238493 is to comment out the
if block at
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/view/src/nsView.cpp&rev=3.210&mark=368-371#365

Comment 11

[Scott MacGregor]

Jackpot! Bug 238493 is indeed the guilty culprit! 

Here's what I did following Boris's suggestion to effectively turn of 238493. I
no longer have the flicker issues. 

Index: nsView.cpp
===================================================================
RCS file: /cvsroot/mozilla/view/src/nsView.cpp,v
retrieving revision 3.210
diff -u -w -r3.210 nsView.cpp
--- nsView.cpp  27 Jan 2005 20:49:46 -0000      3.210
+++ nsView.cpp  26 Feb 2005 06:50:16 -0000
@@ -365,10 +365,10 @@
     // If our view manager has refresh disabled, then do nothing; the view
     // manager will set our position when refresh is reenabled.  Just let it
     // know that it has pending updates.
-    if (!mViewManager->IsRefreshEnabled()) {
-      mViewManager->PostPendingUpdate();
-      return;
-    }
+//    if (!mViewManager->IsRefreshEnabled()) {
+//      mViewManager->PostPendingUpdate();
+//      return;
+//    }

Comment 12

[Boris Zbarsky [:bzbarsky]]

So... Is it possible that we're doing a synchronous paint from somewhere while
we have pending widget updates?  Perhaps break in nsViewManager::ForceUpdate
when mHasPendingUpdates is true and see what the callstack is?

Comment 13

[Scott MacGregor]

Roc, have you had a chance to start looking at this regression yet? Can you
provide some feedback? Thanks!

Comment 14

[Scott MacGregor]

I just sent e-mail to roc directly in case all of my bugzilla attempts to pester
him are getting lost in the bugzilla email shuffle. 

Comment 15

[Scott MacGregor]

Attachment: Patch to disable the optimization until roc has time to fix the regression

I e-mailed roc directly about this bug after and he said he hasn't had time to
look into this yet. Since it's been quite a while and the problem is still
there, I've gone ahead and made a patch which effectively disables the original
optimization from Bug #238493, thus eliminating the regression it caused. 

When Roc has more time to look into it, this change could of course be backed
out.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Scott, the problem is that that's not an optimization.  It's a correctness fix
-- without it, we get very bad flicker during rendering on a wide variety of web
pages out there (especially anything that combines DHTML with any of iframes,
plugins, position:fixed elements, scrollable elements, etc).

This bug should really be in some product where we'd be able to request blocking
1.8b for it (which we should then request), but just backing out the bug 244366
patch is really not an option.

Comment 17

[Gulliver]

It's back again! The flicker was gone in previous days builts but with
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; de-AT; rv:1.8b2) Gecko/20050314
it is back.

Comment 18

[Gulliver]

Hi Scott,

could you please change the Hardware and OS to All. This bug is also present in
the MacOS X version: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; de-AT;
rv:1.8b2) Gecko/20050315

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 177533 [details] [diff] [review]
Patch to disable the optimization until roc has time to fix the regression

I'm not seeing the problem on my Linux GTK1 debug build of the suite. It looks
like I'm in the same boat as Boris.

In theory someone could create a minimized chrome testcase for this.

Comment 20

[Scott MacGregor]

Roc as mentioned before this is really easy to see on Windows and Mac. Look at
the screen shot I attached to this bug to see how obvious it is. By denying the
patch does this mean you have time to help figure out the fix to the regression?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I have time to help, but I'm not sure what I'm going to be able to do.

Well, if IBM delivers the screen I've been waiting for for weeks, I might get a
Windows machine set up sometime this month.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I marked it - because of what bz says in comment 16 ... the fix fixed some
really serious issues, so a straight backout is not an obvious win.

Comment 23

[Martijn Wargers (dead)]

Is this still an issue? It is WFM with 20050411 thunderbird build. (also WFM
with 20050405 build).

Comment 24

[Oliver Saier]

WFM as well on version 1.0+ (20050422) and Win XP home sp2. 

Comment 25

[Scott MacGregor]

I'm not seeing this anymore either! Woo hoo. I don't know why this went away but
I won't complain.