267557 - [FIXr]several websites don't refresh unless I move my mouse

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P1)

Description

[Steve Reynolds]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041103
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041103

Yahoo, Hotmail, Gmail, operamail and our internal email web server are most
noticeable. When I go to these sites, they intially come up properly. But when I
click to read an email, the email doesn't appear unless I move the mouse.

Reproducible: Always
Steps to Reproduce:
1. go to gmail (or some mail system with several emails ready to be read)
2. click on an email, get a blank screen
3. move mouse and the screen appears.

Actual Results:  
nearly blank screen

Expected Results:  
a properly updated screen should appear.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is this a recent regression?  If so, can you pin down when this started?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Steve says this started between Build 2004-11-03-06 and 2004-11-04-06.

Checkins for that range:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-11-03+06%3A00%3A00&maxdate=2004-11-04+06%3A00%3A00&cvsroot=%2Fcvsroot

None of these really jump out at me as being likely to have caused this....

Martijn, can you reproduce this, by any chance?  You have Windows, right?

Comment 3

[Martijn Wargers (dead)]

(In reply to comment #2)
> Martijn, can you reproduce this, by any chance?  You have Windows, right?
I have Windows, but I can't reproduce this bug, using gmail. Used a Mozilla
2004-11-14 build and Firefox 2004-11-15 and 2004-11-04 builds in trying to
reproduce.

Comment 4

[Martijn Wargers (dead)]

Hmm, actually I think I see this same or maybe related problem at
http://www.blogger.com when I add a new blog post. There, I can reproduce the
bug 100% of the time. 

Steps to reproduce:
- Go to http://www.blogger.com/start
- Sign in at your account (if you don't have any, you have to create one)
- Click on the "+" under "New Post"
- Press "Publish Post". After that don't move you mouse at all.
After that the bottom page is blank, until you move the mouse. After that you
get to see "Your blog published successfully." etc.

This seems to be the regression range for me for this issue:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041030
Firefox/0.9.1+ 8:18am
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041031
Firefox/0.9.1+ 7:39am

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-10-30+08%3A00%3A00&maxdate=2004-10-31+08%3A00%3A00&cvsroot=%2Fcvsroot

Comment 5

[Martijn Wargers (dead)]

Forget about the most of comment 4 (except the regression range perhaps). I
think I'll be able to make a testcase which shows the bug. Hang on for a moment.

Comment 6

[Martijn Wargers (dead)]

Attachment: Testcase

This testcase shows the bug 100% reproducable for me. 
Two characteristics that are needed to trigger the bug:
- The bugged page has to be in a (i)frame.
- The bugged page has to have an image inside it.

Comment 4 is probably totally ****. With the testcase I get this regression
range:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041029
Firefox/0.9.1+ 7:22am
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041030
Firefox/0.9.1+ 8:18am
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-10-29+07%3A00%3A00&maxdate=2004-10-30+09%3A00%3A00&cvsroot=%2Fcvsroot

Comment 7

[Boris Zbarsky [:bzbarsky]]

OK, this looks like a regression from bug 244290.  The iframe is needed so we
hav a viewmanager tree; the image, I suspect, causes us to actually make use of
the paint suppression timer or something....

I can reproduce this reliably without the mouse thing if I disable the
nsEventStateManager::FlushPendingEvents stuff that runs on mouse moves (that is,
if I do that the iframe simply never paints).

Martijn, thank you for the testcase!

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK, I tracked this about a bit.  One issue I see offhand is the DisableRefresh()
call on the viewmanager in PresShell::Destroy.  This disabled refresh on the
entire viewmanager tree, and I see no corresponding EnableRefresh().

I bet if the refresh thing were a counter, not a boolean, it would never get
reenabled at all.  As it is, the next time we do a disable/enable cycle it gets
reenabled.

Robert, I'm really not quite sure what the best solution here is.  I really
don't like having the EnableRefresh/DisableRefresh api...

Perhaps the presshell should use a view update batch in ::Destroy?  As long as
we're not unsetting the mRootViewManager pointer (bug 269402) that should be
ok...  Alternately, we could just EnableRefresh() here.

I _would_ like to make the refresh thing a counter, to better catch errors like
this, however.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Oh, so to make it clear, if I comment out the DisableRefresh in
PresShell::Destroy it fixes things on the testcase for me.

Comment 10

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

> Robert, I'm really not quite sure what the best solution here is.  I really
> don't like having the EnableRefresh/DisableRefresh api...

Agreed.

I traced the DisableRefresh call in PresShell::Destroy to see when and why it
was introduced. It was originally added to PresShell::~PresShell() by kipp; the
CVS comment just says "3.125 <[e-mail]> 1999-04-29 17:08 Disable
rendering during presentation tear down". Sigh. It clearly isn't the right thing
to do when Enable/DisableRefresh is global. I say we take it out and see what
the damage is.

Comment 11

[Boris Zbarsky [:bzbarsky]]

The other option is to re-enable refresh... I'm not sure which is more dangerous
for 1.8a5...

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to reenable refresh

I checked that this also works....

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 166416 [details] [diff] [review]
Patch to reenable refresh

Robert, if you do prefer that I just take out the DisableRefresh, let me know
and I will.

Comment 14

[Mats Palmgren (inactive)]

The patch does not seem to fix bug 270925. The bug also occurs on several
sites that uses <frame>s, http://www.cdon.se for example (the patch did not
fix those either).

Comment 15

[Mats Palmgren (inactive)]

The patch does not fix the testcase either for me...

Comment 16

[Boris Zbarsky [:bzbarsky]]

See comment 8 -- this patch depends on the patch in bug 269402.

Comment 17

[Mats Palmgren (inactive)]

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

Comment 18

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

> Robert, if you do prefer that I just take out the DisableRefresh, let me know
> and I will.

I think we should do that and find out why it's needed :-). If it is needed then
we fix the underlying problem. r+sr for that.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to just remove DisableRefresh() call

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 166720 [details] [diff] [review]
Patch to just remove DisableRefresh() call

This has reviews from Robert, and  fixes a bunch of painting regressions on
pages with frames.  It's reasonably safe as changes go, I believe.

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 166720 [details] [diff] [review]
Patch to just remove DisableRefresh() call

a=asa for 1.8a5 checkin.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.