301338 - Alert boxes (sheets) prevent window loading/painting

Status: VERIFIED FIXED

(Core Graveyard :: Widget: Mac, defect)

Description

[Stefan [:stefanh]]

Mac OS 10.3.9, Seamonkey trunk 2005071711

If I start mailNews and the alert box "Do you wish to compact all folders to
save disk space" appear, the alert box seems to prevent the upper part (toolbar)
to load or paint properly. After the box has been dismissed, large portions of
the toolbar are completely blank. Hovering over the affected area with the mouse
partly restores it. 

First seen: Mozilla trunk 2005062209
Does not occur with Mozilla trunk 2005062109

Bonsai link:
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=2005-06-21+09%3A00%3A00&maxdate=2005-06-22+09%3A00%3A00&cvsroot=%2Fcvsroot


I'm not sure, but I think this could be caused by the fix for bug 282940
(CFRunLoopSource), hence the component.

Comment 1

[Stefan [:stefanh]]

Attachment: Screenshots of the issue

From top to bottom:
1) Initial load, alert box appear
2) After box is dismissed
3) Result of hovering over parts of the toolbar

Comment 2

[Stefan [:stefanh]]

fwiw, this also occurs in the thunderbird nightly builds I've tested: 050719 and
050629 (no problem with a TB nightly from 050620).

Comment 3

[Simon Fraser [no longer active]]

I wonder if the patch in bug 281455 (which moves window updates to Carbon
Events) would help here.

Comment 4

[Simon Fraser [no longer active]]

Bug 281455 is fixed. Does this still happen (sorry, I don't have a tbird build).

Comment 5

[Gary Elshaw]

I can't replicate this in Mozilla 1.7.10 or Thunderbird 1.0.6 on OS X 10.4.2

Comment 6

[Stefan [:stefanh]]

(In reply to comment #5)
> I can't replicate this in Mozilla 1.7.10 or Thunderbird 1.0.6 on OS X 10.4.2

This bug doesn't occur on the 1.7 branch (Mozilla 1.7.10, Thunderbird 1.0.6), it
only occurs on the trunk.So, you have to use a trunk build (after July 20) in
order to test if it's fixed or not.

Comment 7

[Stefan [:stefanh]]

Hmm, I was actually able to reproduce this one time in SeaMonkey 2005072110. But
now i can't reproduce it... Odd.

Comment 8

[Stefan [:stefanh]]

(In reply to comment #7)
> Hmm, I was actually able to reproduce this one time in SeaMonkey 2005072110. But
> now i can't reproduce it... Odd.

Well, it's still there in SeaMonkey 2005072110. I can still reproduce it, but 
it's not as easy as it was before. Then you just needed to open mailNews. I'm
able to reproduce it if I load a page that isn't cached and open mailnews at the
same time.

Comment 9

[Asa Dotzler [:asa]]

Josh tells me this isn't a problem in Firefox so I'm minusing the blocking request.

Comment 10

[Asaf Romano (gone)]

yes, this is fixed now.

Comment 11

[Stefan [:stefanh]]

I still see this in SeaMonkey (2005072511) and in the latest Thunderbird trunk
build (20050726).

Comment 12

[Simon Fraser [no longer active]]

Is there a way to reproduce this in Firefox?

Comment 13

[Asaf Romano (gone)]

Either Cmd+D during page load, or, make Camino/Safari your defualt browser and
launch Firefox.

Comment 14

[Asaf Romano (gone)]

Renominating - this is causing UI bustages in both Firefox and Thundebird.

Comment 15

[Mark Mentovai]

I see this too.  The area not being painted has the same dimensions as the
sheet, but with its left edge fixed to the window's own left.  (Probably top,
too.)  The sheet's own mBounds seem reasonable.

Comment 16

[Asa Dotzler [:asa]]

Josh or Simon, have either of you had a chance to dig into this? 

Comment 17

[Mark Mentovai]

-> me

Comment 18

[Mark Mentovai]

Really, I mean it.

Comment 19

[Mark Mentovai]

Attachment: Don't clip out top-level windows

We don't store the sheet's offset relative to its parent window, because the
system always centers it horizontally at the top of its parent.  We call the
position (0,0).  When it comes time to clip, the clipping gets done at (0,0). 
That's why the non-updating region is the size of the sheet, but at the top
left of the window.  Even if the sheet's bounds were fixed up to correspond to
its position relative to its parent, the rect that had been beneath the sheet
would be stale when the sheet disappears.

Sheets are top-level windows (so are dialogs), so they don't need to be clipped
out like this.	Other top-level windows aren't clipped, but that's because
they're not siblings of widgets that aren't top-level windows, and this
clipping is only done if the widget itself isn't a top-level window.  Sheets
are anomalous in this regard.

With this patch, widgets that aren't top-level windows won't clip out sheets or
any other top-level windows that they might find themselves being siblings of.

Comment 20

[Simon Fraser [no longer active]]

Is this really CFRunLoop fallout? It seems more likely to be caused by some
widget hierarchy changes.

I'm rather surprised that sheets live in the widget hierarchy of their parent
windows, but I guess this is how Win32 works.

Anyway, I'd like to know what caused this to regress.

Comment 21

[Mark Mentovai]

Not CFRunloop fallout.  This regressed between 031408 and 031509.

Regression window:
http://bonsai.mozilla.org/cvsquery.cgi?branch=HEAD&date=explicit&mindate=2005-03-14+05%3A00&maxdate=2005-03-15+09%3A00

Comment 22

[Stefan [:stefanh]]

(In reply to comment #21)
> Not CFRunloop fallout.  This regressed between 031408 and 031509.
> 
> Regression window:
>
http://bonsai.mozilla.org/cvsquery.cgi?branch=HEAD&date=explicit&mindate=2005-03-14+05%3A00&maxdate=2005-03-15+09%3A00

Sorry for the confusion, but I was 100% convinced of my regression range... :(

Comment 23

[Josh Aas]

Comment on attachment 196977 [details] [diff] [review]
Don't clip out top-level windows

Seems to fix the problem, which I was easily able to reproduce in a non-patched
Firefox 1.5b1 today. My only nit about the patch is that the function name
"IsTopWidgetWindow" could be a little more clear (include the concept of
"top-level", not just "top"). Yeah, I know it is the variable name, but they
don't have to match, or you can fix both. Just a nit, fine with me if you
choose not to change it.

Comment 24

[Simon Fraser [no longer active]]

Comment on attachment 196977 [details] [diff] [review]
Don't clip out top-level windows

>cvs diff -u8p mozilla/widget/src/mac/nsChildWindow.cpp mozilla/widget/src/mac/nsWindow.h
>
>Index: mozilla/widget/src/mac/nsChildWindow.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/widget/src/mac/nsChildWindow.cpp,v
>retrieving revision 1.18
>diff -u -8 -p -r1.18 nsChildWindow.cpp
>--- mozilla/widget/src/mac/nsChildWindow.cpp	22 May 2004 20:54:39 -0000	1.18
>+++ mozilla/widget/src/mac/nsChildWindow.cpp	22 Sep 2005 01:01:33 -0000
>@@ -92,40 +92,40 @@ nsresult nsChildWindow::StandardCreate(n
> 
> void nsChildWindow::CalcWindowRegions()
> {
> 	Inherited::CalcWindowRegions();
> 
> 	// clip the siblings out of the window region and visRegion 
> 	if (mClipSiblings && mParent && !mIsTopWidgetWindow) {
> 		// need to walk the siblings backwards, to get clipping right.
>-		nsIWidget* sibling = mParent->GetLastChild();
>+		nsWindow* sibling = NS_STATIC_CAST(nsWindow*, mParent->GetLastChild());
> 		if (!sibling)
> 			return;
> 
> 		StRegionFromPool siblingRgn;
> 		if (siblingRgn == nsnull)
> 			return;
> 
> 		do {
>-			if (sibling == NS_STATIC_CAST(nsIWidget*, this))
>+			if (sibling == NS_STATIC_CAST(nsWindow*, this))
> 				break;
> 
> 			PRBool visible;
> 			sibling->IsVisible(visible);
>-			if (visible) {	// don't clip if not visible.
>+			if (visible && !sibling->IsTopWidgetWindow()) {	// don't clip if not visible or if a top-level window.
> 				// get sibling's bounds in parent's coordinate system.
> 				nsRect siblingRect;
> 				sibling->GetBounds(siblingRect);
> 					
> 				// transform from parent's coordinate system to widget coordinates.
> 				siblingRect.MoveBy(-mBounds.x, -mBounds.y);
> 
> 				Rect macRect;
> 				::SetRect(&macRect, siblingRect.x, siblingRect.y, siblingRect.XMost(), siblingRect.YMost());
> 				::RectRgn(siblingRgn, &macRect);
> 				::DiffRgn(mWindowRegion, siblingRgn, mWindowRegion);
> 				::DiffRgn(mVisRegion, siblingRgn, mVisRegion);
> 			}
>-			sibling = sibling->GetPrevSibling();
>+			sibling = NS_STATIC_CAST(nsWindow*, sibling->GetPrevSibling());
> 		} while (sibling);
> 	}
> }

You might as well detab the whole method while you're there.

>Index: mozilla/widget/src/mac/nsWindow.h
>===================================================================

>+    PRBool              IsTopWidgetWindow() { return mIsTopWidgetWindow; };

Make this method "|const|.

Comment 25

[Mark Mentovai]

Fixed on trunk.

Comment 26

[Chris Lawson (gone)]

Is this at all related to the problem in Camino where a sheet pops up (say, a
cookie confirmation dialog) and prevents any page content from being drawn or
loaded until the dialog sheet is dismissed?

cl

Comment 27

[Simon Fraser [no longer active]]

(In reply to comment #26)
> Is this at all related to the problem in Camino where a sheet pops up (say, a
> cookie confirmation dialog) and prevents any page content from being drawn or
> loaded until the dialog sheet is dismissed?

No.

Comment 28

[Tracy Walker [:tracy]]

Stephan, please verify this is fixed with the most recent trunk nightly build. 
Thanks

Comment 29

[Stefan [:stefanh]]

Verified fixed with SeaMonkey trunk 2005092310. Thanks :)

Comment 30

[Mark Mentovai]

Bixed on franch.

Comment 31

[Scott MacGregor]

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