Closed Bug 121583 Opened 23 years ago Closed 23 years ago

Some windows (Mail, Venkman) freak out with 100% CPU on window resize (when no height on an element)

Categories

(Core :: Layout, defect, P1)

Product:
Core
Component:
Layout
Platform:
x86
All
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: caillon, Assigned: hyatt)

References

Details

(Keywords: crash, hang, Whiteboard: [adt1][m5+] [driver:asa] [ETA 05/03] [Needs a=])

Attachments

(12 files)

Stack trace of breaking in the debugger
4.92 KB, text/plain
Details
stopgap patch: prevent the tree control from getting too small (force min-width to 40px)
473 bytes, patch
Details | Diff | Splinter Review
Some helpful stack traces and a pointer to the xbl involved.
11.49 KB, text/plain
Details
nsTreeBodyFrame.cpp Patch (Make sure page count is always 1 if we have at least 1 row.)
739 bytes, patch
Details | Diff | Splinter Review
Patch to fix bug
889 bytes, patch
bryner
: review+
Details | Diff | Splinter Review
testcase
714 bytes, application/vnd.mozilla.xul+xml
Details
Patch to give a treebody's scrollbar a preferred height of 0, to match the preferred height of 0 we also give (in C++) for the treebody.
680 bytes, patch
bugs
: review+
kinmoz
: superreview+
Details | Diff | Splinter Review
Prevent the minsizes of the buttons and thumb from coming into play by giving them all minwidths/heights of 0.
2.03 KB, patch
Details | Diff | Splinter Review
Branch only fix
826 bytes, patch
mscott
: review+
mscott
: superreview+
asa
: approval+
Details | Diff | Splinter Review
Proposed trunk patch
4.45 KB, patch
Details | Diff | Splinter Review
Test case
3.21 KB, application/vnd.mozilla.xul+xml
Details
you can actually drag the vertical separator to the left till it crossed the scrollbar of the mail folder listbox
5.14 KB, image/x-png
Details 
On linux 2002012308 and a CVS build from ~2hrs ago.

If you have a couple of files in Venkman's script view (with the filenames) and
resize Venkman's main window in small increments (enlarge it if you have
scrollbars and shrink it if you don't have scrollbars) such that you resize it
into a state where it is very close to to either having scrollbars or not, it
appears that it gets confused and rapidly alternates between scrollbars and no
scrollbars.  Venkman returns to normal when you load additional scripts and the
script view refreshes.

This seems to ignore when you resize the window by drastic amounts.
Unfortunately it is not always easy to resize it just the right amount, but if
you play with resizing the window back and forth around where the scrollbars
would come or go, you will eventually hit it. There might be some error in
calculation of pane height which causes this to happen.

Also note that so far I have only been able to reproduce this while using the
modern theme.

rginda says Layout so filing there.
Changing QA contact
QA Contact: petersen → amar
 Reporter can you please provide a testcase for the problem you have described.

There is no "testcase" -- I've given the steps to reproduce.  I can try and make
it clearer though with a certain path if you like.

1. Start Venkman with mozilla -venkman
2. Go to Edit > Prefs.  This loads an average amount of files into the scripts
pane in venkman.
3. If the scripts pane currently has a scrollbar, resize Venkman larger in small
increments so that you eventually have no scrollbar.  If there is no scrollbar,
resize Venkman's window smaller.

Note: the purpose of this is to resize the Venkman window so that the scripts
pane is in a state between needing a scrollbar and not.  It is not easy to
reproduce but when you do reproduce it, it is obvious.  And I can reproduce this
100% of the time if I keep trying.  It sometimes takes me a few minutes to
resize Venkman to the right spot, sometimes it takes a few seconds.

But if you keep resizing Venkman in small increments back and forth so that the
scripts panel is right where it can have a scrollbar or not, it starts going
crazy.  I am sorry that the description is poor here, but this is the best I can
do to describe it.
Target Milestone: --- → mozilla1.1
 I think this bug is related to Javascript debugger. Fowarding this to 
javascript debugger and changing the QA contact.
Component: Layout → JavaScript Debugger
QA Contact: amar → caillon
This is related to the javascript debugger in that the debugger has XUL that
causes this layout loop.  The debugger is made strictly from javascript and XUL,
however.  There are no onresize handlers defined in the XUL.  Marc, what do you
think?
Marc, comments please?

As it turns out, this is very easy to reproduce.  Open venkman, grab the only
horizontal splitter, and drag it slowly twords the top of the window.  As you
approach the top layout begins to oscillate between two states.  If you let go
of the splitter while in that region, layout will continue to switch between two
states.

I have no resize handlers, so I don't see how it could be my problem.

A number of users have reported this.  Some of them see this behavior on startup.
> Open venkman, grab the only horizontal splitter ...

I misspoke.  There are two horizontal splitters, the one I'm refering to splits
the entire window, just above the black "console view".


*** Bug 124451 has been marked as a duplicate of this bug. ***
marc?
Sorry, I ignore most bug mails, especially ones with TM > 1.0 :-D  It is a
survival tactic, actually (check my bug count to see what I mean).

I'd need to dig into this.  I cannot think of anything specifically that could
cause it, but I'd be interested in seeing what the stack loks like if you break
into the debugger during the nasty part.  Is this theme-specific?  Does it
happen on any other platforms? At other screen resolutions?  Maybe it is a
simple rounding error...
I've been able to reproduce this with both Classic and Modern so it's not theme
specific.  It's not OS specific either as there are reports from Win32 and I'd
bet it's probably reproducible on Mac too (but not sure).
OS: Linux → All
*** Bug 122153 has been marked as a duplicate of this bug. ***
Marc: Okay so it appears the problem then is related to no height being set.   
This affects more than just Venkman.  See duped bug 122153 for an alternate way
to reproduce this.

Changing summary to hopefully make this bug easier to find and nominating for 1.0
Component: JavaScript Debugger → Layout
Keywords: mozilla1.0
Summary: Venkman script view freaks out with 100% CPU on window resize → Some windows (Mail, Venkman) freak out with 100% CPU on window resize (when no height on an element)
Venkman's problem seems to be that the top outliner has exactly one element in
it.  When the sliders are adjusted so that the single element is no longer
completely visible, layout starts to oscillate.

To see this in addressbook, create a new profile, start address book, and create
a single entry.  Drag the splitter (don't let go!) so that the element is
partially hidden and stay there.  You should see the problem.
Add a second addressbook entry and try again, and you'll notice that scrollbars
appear, which enforce a min-height before you can cause any trouble.


I'll added outliner { min-height: 50px } to venkman.css as a work around.
*** Bug 125467 has been marked as a duplicate of this bug. ***
*** Bug 127478 has been marked as a duplicate of this bug. ***
I tried breaking 5 or 6 times, all the stacks were essentially like the one I
just attached, though usually not as deep.  So it looks like we're spending all
our time updating views!  Is this a view problem, not a layout one?

I can reproduce this at will in page info (eg bug 127278) so if there is any
more information I can provide....

I'm on Linux, Modern theme, 1152x864 resolution.
It's doing a repaint. AFAIK the view system doesn't have any major performance
issues with repainting. Of course someone could be requesting too many paints.

Try turning on paint flashing in a debug build to see what is being painted and
how often.
*** Bug 127900 has been marked as a duplicate of this bug. ***
*** Bug 130024 has been marked as a duplicate of this bug. ***
*** Bug 130662 has been marked as a duplicate of this bug. ***
I just saw this in the address book window on Mac OS 9, in 2002022708.
*** Bug 133297 has been marked as a duplicate of this bug. ***
*** Bug 136323 has been marked as a duplicate of this bug. ***
*** Bug 135304 has been marked as a duplicate of this bug. ***
according to http://bugzilla.mozilla.org/show_bug.cgi?id=135304#c1 this can also
cause a crash
*** Bug 137416 has been marked as a duplicate of this bug. ***
*** Bug 136930 has been marked as a duplicate of this bug. ***
moving bits over from 136930
Blocks: 134771
Keywords: mozilla1.0 → mozilla1.0+, nsbeta1+
Keywords: crash
Whiteboard: [adt1]
any updates on this one?
I just started looking at this bug today. I cannot reproduce the Venkman
problem, but I can reproduce the address book version of the bug.

I find that the problem is with the XUL Element being set to collapsed, then
not-collapsed, then collapsed again, and so on. Here is a stack leading up to
the nsXULElement::SetCollapsed call:

nsXULElement::SetCollapsed(nsXULElement * const 0x03ed0e1c, int 1) line 4152
XPTC_InvokeByIndex(nsISupports * 0x03ed0e1c, unsigned int 65, unsigned int 1,
nsXPTCVariant * 0x0012d6a8) line 106
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_SETTER) line 2025 + 42 bytes
XPCWrappedNative::SetAttribute(XPCCallContext & {...}) line 1826 + 14 bytes
XPC_WN_GetterSetter(JSContext * 0x03f959e0, JSObject * 0x039f6fc8, unsigned int
1, long * 0x05287164, long * 0x0012d96c) line 1290 + 12 bytes
js_Invoke(JSContext * 0x03f959e0, unsigned int 1, unsigned int 2) line 788 + 23
bytes
js_InternalInvoke(JSContext * 0x03f959e0, JSObject * 0x039f6fc8, long 60558272,
unsigned int 0, unsigned int 1, long * 0x0012e294, long * 0x0012e294) line 880 +
20 bytes
js_SetProperty(JSContext * 0x03f959e0, JSObject * 0x039f6fc8, long 49493984,
long * 0x0012e294) line 2590 + 47 bytes
js_Interpret(JSContext * 0x03f959e0, long * 0x0012e414) line 2587 + 1751 bytes
js_Invoke(JSContext * 0x03f959e0, unsigned int 1, unsigned int 2) line 805 + 13
bytes
js_InternalInvoke(JSContext * 0x03f959e0, JSObject * 0x039f6fc0, long 87322712,
unsigned int 0, unsigned int 1, long * 0x0012e674, long * 0x0012e544) line 880 +
20 bytes
JS_CallFunctionValue(JSContext * 0x03f959e0, JSObject * 0x039f6fc0, long
87322712, unsigned int 1, long * 0x0012e674, long * 0x0012e544) line 3412 + 31 bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x03f95968, void * 0x039f6fc0,
void * 0x05347058, unsigned int 1, void * 0x0012e674, int * 0x0012e678, int 0)
line 1016 + 33 bytes
nsJSEventListener::HandleEvent(nsJSEventListener * const 0x0528dd58, nsIDOMEvent
* 0x0528d020) line 180 + 77 bytes
nsXBLPrototypeHandler::ExecuteHandler(nsXBLPrototypeHandler * const 0x03b21330,
nsIDOMEventReceiver * 0x03f42ad8, nsIDOMEvent * 0x0528d020) line 447
nsXBLScrollHandler::Underflow(nsXBLScrollHandler * const 0x03ede500, nsIDOMEvent
* 0x0528d020) line 116
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x03ede650,
nsIPresContext * 0x04f7cb40, nsEvent * 0x0528a5f8, nsIDOMEvent * * 0x0012fba4,
nsIDOMEventTarget * 0x03f42ad8, unsigned int 2, nsEventStatus * 0x0012fbe4) line
2061 + 41 bytes
nsXULElement::HandleDOMEvent(nsXULElement * const 0x03f42ad0, nsIPresContext *
0x04f7cb40, nsEvent * 0x0528a5f8, nsIDOMEvent * * 0x0012fba4, unsigned int 2,
nsEventStatus * 0x0012fbe4) line 3464
nsXULElement::HandleDOMEvent(nsXULElement * const 0x03ed0bd8, nsIPresContext *
0x04f7cb40, nsEvent * 0x0528a5f8, nsIDOMEvent * * 0x0012fba4, unsigned int 2,
nsEventStatus * 0x0012fbe4) line 3481 + 50 bytes
nsXULElement::HandleDOMEvent(nsXULElement * const 0x03bb6ea8, nsIPresContext *
0x04f7cb40, nsEvent * 0x0528a5f8, nsIDOMEvent * * 0x0012fba4, unsigned int 1,
nsEventStatus * 0x0012fbe4) line 3481 + 50 bytes
PresShell::HandlePostedDOMEvents() line 4994
PresShell::ProcessReflowCommands(int 1) line 6419
ReflowEvent::HandleEvent() line 6202
HandlePLEvent(ReflowEvent * 0x0528d1c0) line 6216
PL_HandleEvent(PLEvent * 0x0528d1c0) line 596 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x0027a0f8) line 526 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x001e0c38, unsigned int 49384, unsigned int 0,
long 2597112) line 1077 + 9 bytes
USER32! 77e12e98()
USER32! 77e130e0()
USER32! 77e15824()
nsAppShellService::Run(nsAppShellService * const 0x01a01658) line 309
main1(int 2, char * * 0x00277840, nsISupports * 0x00000000) line 1414 + 32 bytes
main(int 2, char * * 0x00277840) line 1762 + 37 bytes
mainCRTStartup() line 338 + 17 bytes

I'm not sure why the script is toggling the collapsed state, but the rest of
this mess is just layout trying to keep up with the change in state I think.

This looks like a XUL issue to me, anybody think otherwise?
For what it's worth, the code that is causing the collapsing / uncollapsing is
coming from the reflow of the TreeBodyFrame. In there it posts an event with
message == NS_SCROLLPORT_[OVERFLOW|UNDERFLOW], in the context of this stack:

nsTreeBodyFrame::CheckVerticalOverflow(int 1) line 922
nsTreeBodyFrame::SetBounds(nsTreeBodyFrame * const 0x03d90afc, nsBoxLayoutState
& {...}, const nsRect & {...}) line 603
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01bbefc0, nsIBox *
0x03d90a20, nsBoxLayoutState & {...}) line 493
nsContainerBox::DoLayout(nsContainerBox * const 0x03d90a20, nsBoxLayoutState &
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x03d90a20, nsBoxLayoutState & {...})
line 1208
nsBox::Layout(nsBox * const 0x03d90a20, nsBoxLayoutState & {...}) line 1052
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01bbefc0, nsIBox *
0x03d9096c, nsBoxLayoutState & {...}) line 528
nsContainerBox::DoLayout(nsContainerBox * const 0x03d9096c, nsBoxLayoutState &
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x03d9096c, nsBoxLayoutState & {...})
line 1208
nsBox::Layout(nsBox * const 0x03d9096c, nsBoxLayoutState & {...}) line 1052
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01bbefc0, nsIBox *
0x05017358, nsBoxLayoutState & {...}) line 528
nsContainerBox::DoLayout(nsContainerBox * const 0x05017358, nsBoxLayoutState &
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x05017358, nsBoxLayoutState & {...})
line 1208
nsBox::Layout(nsBox * const 0x05017358, nsBoxLayoutState & {...}) line 1052
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01bbefc0, nsIBox *
0x03ecf128, nsBoxLayoutState & {...}) line 528
nsContainerBox::DoLayout(nsContainerBox * const 0x03ecf128, nsBoxLayoutState &
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x03ecf128, nsBoxLayoutState & {...})
line 1208
nsBox::Layout(nsBox * const 0x03ecf128, nsBoxLayoutState & {...}) line 1052
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01bbefc0, nsIBox *
0x03f3bca8, nsBoxLayoutState & {...}) line 528
nsContainerBox::DoLayout(nsContainerBox * const 0x03f3bca8, nsBoxLayoutState &
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x03f3bca8, nsBoxLayoutState & {...})
line 1208
nsBox::Layout(nsBox * const 0x03f3bca8, nsBoxLayoutState & {...}) line 1052
nsSprocketLayout::Layout(nsSprocketLayout * const 0x01bbefc0, nsIBox *
0x0501b090, nsBoxLayoutState & {...}) line 528
nsContainerBox::DoLayout(nsContainerBox * const 0x0501b090, nsBoxLayoutState &
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x0501b090, nsBoxLayoutState & {...})
line 1208
nsBox::Layout(nsBox * const 0x0501b090, nsBoxLayoutState & {...}) line 1052
nsStackLayout::Layout(nsStackLayout * const 0x01baa028, nsIBox * 0x0501ad98,
nsBoxLayoutState & {...}) line 331
nsContainerBox::DoLayout(nsContainerBox * const 0x0501ad98, nsBoxLayoutState &
{...}) line 606 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x0501ad98, nsBoxLayoutState & {...})
line 1208
nsBox::Layout(nsBox * const 0x0501ad98, nsBoxLayoutState & {...}) line 1052
nsBoxFrame::Reflow(nsBoxFrame * const 0x0501ad60, nsIPresContext * 0x04f7cb40,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 1001
nsRootBoxFrame::Reflow(nsRootBoxFrame * const 0x0501ad60, nsIPresContext *
0x04f7cb40, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 243
nsContainerFrame::ReflowChild(nsIFrame * 0x0501ad60, nsIPresContext *
0x04f7cb40, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0,
int 0, unsigned int 0, unsigned int & 0) line 785 + 31 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x0501ad24, nsIPresContext *
0x04f7cb40, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 588
nsHTMLReflowCommand::Dispatch(nsIPresContext * 0x04f7cb40, nsHTMLReflowMetrics &
{...}, const nsSize & {...}, nsIRenderingContext & {...}) line 222
PresShell::ProcessReflowCommand(nsVoidArray & {...}, int 1, nsHTMLReflowMetrics
& {...}, nsSize & {...}, nsIRenderingContext & {...}) line 6291
PresShell::ProcessReflowCommands(int 1) line 6346
ReflowEvent::HandleEvent() line 6202
HandlePLEvent(ReflowEvent * 0x05290530) line 6216
PL_HandleEvent(PLEvent * 0x05290530) line 596 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x0027a0f8) line 526 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x001e0c38, unsigned int 49384, unsigned int 0,
long 2597112) line 1077 + 9 bytes
USER32! 77e12e98()
USER32! 77e130e0()
USER32! 77e15824()
nsAppShellService::Run(nsAppShellService * const 0x01a01658) line 309
main1(int 2, char * * 0x00277840, nsISupports * 0x00000000) line 1414 + 32 bytes
main(int 2, char * * 0x00277840) line 1762 + 37 bytes
mainCRTStartup() line 338 + 17 bytes


I really need to learn BOX layout better...
*** Bug 137749 has been marked as a duplicate of this bug. ***
Bug 137416 is a duplicate but it worth a look because it shows how to
systematically reproduce the bug in the Mail module.
The easiest way to reproduce this is to set View/Headers in MailNews to all and
to open the header of a mail with an attachement. Do we know what caused this
and could we check that out? It makes MailNews unusable.
Accepting as [P1 - Moz1.0] bug until somebody with more Box experience steals it
from me...
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: mozilla1.1alpha → mozilla1.0
Marc, this might help.  There is a decent explaination of what I've found here:

http://bugzilla.mozilla.org/show_bug.cgi?id=136930#c9

It might help.
Thanks Chris. I think I have that much of it figured out now. The problem I am
having is determining just where the 10px (150 twips) difference in the height
of the InnerBox of the TreeBodyFrame is coming from...
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Uhh..why is this marked fix?
wishful thinking? More likely accidental change... REOPENING
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Accepting (again)
Status: REOPENED → ASSIGNED
Here is a stopgap fix that might be worth thinking about until the box layout
(reflow) can be untangled. Unfortunately, I don't see much in the way of
debugging aids in box-land, and I am a total box-newbie, so it could take a
while.

The patch prevents the hang by preventing the race condition that happens when
the height gets smaller than a single rowitem and a scrollbar is required. This
is essentially what others have done on specific uses of the tree control. I
tested this with the address book hang - the only one I can get to happen.
typo in last comment, sorry. The stopgap patch forces the min-height to 40px,
not the min-width.

We should consider using this until a Box-dude can fix the underlying problem, I
am getting nowhere on it so far.

What is the urgency for this bug anyway? It is marked as blocking RC1, but I
thought that was a done-deal already...
Sorry, should be blocking RC2 as well.  Marc, who should be looking at this to
get the right fix in?
Blocks: 138000
Here are some stack traces and a pointer to the tree XBL code that's being
triggered. Hopefully, this along with the stack that attinasi posted above, can
provide more detail to whoever is able to fix this.

The short version of this, which blizzard and attinasi have pointed out, is
that the nsTreeBodyFrame code posts a DOM event during a reflow. This DOM event
gets processed after the reflow, and triggers some xbl which triggers box code
that posts another reflow that starts the whole mess over.

Don't pay any attention to the pointer addresses used in the stack frames,
since the 2 stacks are from 2 different breaks ... the stack traces are always
the same though.
I wonder why the height of the tree is increased after showing the scrollbar.
I think this is the reason why it dies in endless loop. (we need scrollbar, so
we show it, but scrollbar increases the height, and then we have enough space
for rows so we hide scrollbar again and then again and again ...

This patch prevents the infinite loop for me, by preventing mPageCount from
ever being zero if we have at least 1 row.
It seems that this is not limited only to the special case when there is 1 row.
I can reproduce the infinite loop in the Addresbook even with this patch.

Ok, I'm seeing an infinite loop still too, but only if I resize the outer window 
of the address book so that it causes the tree frame to resize vertically too.

The strange thing is that the infinite loop stops if I Alt-Tab to a different 
window. I can't just click in another window because the windows cursor is still 
stuck in resize mode. After I Alt-Tab I can go back to address book and 
everything is fine?

Still investigating.
Kin, I tried that forcing the page count to 1 solution too, but it fails when
there are more than one rows in the tree - as Jan pointed out, this is not
limited to the case where there is 1 row.

I think hewitt might be able to help, so CC'ing him...
Jan has hit the problem on the head in comment 47, in my skin the minimum tree
height with column headers and a scroll bar is 54px, so I can demonstrate the
problem in Venkman even with the workaround that sets the minimum height to 50px.

I've got a truly lame hack for this in CSS but the other way to fix this in XUL
would be to make visibility: collapse; only collapse in the parent box direction.

I mention that only because that also fixes a problem setting collapsed="true"
on rows and columns in grids: they can't seem to recalculate their size correctly.
*** Bug 139061 has been marked as a duplicate of this bug. ***
*** Bug 138569 has been marked as a duplicate of this bug. ***
*** Bug 123813 has been marked as a duplicate of this bug. ***
Ho hum, not so simple, my idea makes autocomplete look weird :-(
*** Bug 139227 has been marked as a duplicate of this bug. ***
For me this bug appeared first in 0.9.8. Perhaps a change between 0.9.7 and 
0.9.8 has caused this?
Blocks: 139415
I think Hyatt's back - let's have him look at it now.

David, can you please see if you understand why the Tree goes all loopy when the
size is too small?
Assignee: attinasi → hyatt
Status: ASSIGNED → NEW
Marc, have you not been reading previous comments? Jan pointed out that when the
scrollbar is shown it increases the height of the tree, thus confusing it into
thinking that a scrollbar is unnecessary. The tree therefore removes the
scrollbar thus reducing the height of the tree, making a scrollbar necessary,
etc, etc.
> Marc, have you not been reading previous comments? 

Um, yes, I have been reading them. And there is really no need to be
condescending, just because I gave the bug to the author of the code in question.

>Jan pointed out that when the scrollbar is shown it increases the height of the
>tree, thus confusing it into thinking that a scrollbar is unnecessary. The tree
>therefore removes the scrollbar thus reducing the height of the tree, making a
>scrollbar necessary, etc, etc.

Thanks for reiterating what Jan already clearly stated. Still, knowing that has
not helped me to find the fix though.

OK Marc, maybe I was a bit harsh. Your "all loopy" confused me.
I think I've seen something similar in Mailnews. In one of my inboxes, I have
just enough messages to force a scrollbar to be drawn in the message list.
However when the scrollbar is drawn, moz decides there is enough space without
the scrollbar and 'undraws' it. This causes the separator between the message
list and the message pane to 'wobble' up and down quite nauseatingly.

Marc are you still working on this?  Should we reassign it back to you?
Whiteboard: [adt1] → [adt1][m5+]
I reassigned it to Hyatt so he could work on it. I'm no longer looking at it. Is
there anyone else who knows Box if Hyatt is unable to work on it? 
I don't know who is working on it but someone should - it's an RC2 bug and it's
a really bad bug, especially on windows where it essentially hangs mail/news.
*** Bug 140223 has been marked as a duplicate of this bug. ***
*** Bug 137634 has been marked as a duplicate of this bug. ***
*** Bug 136520 has been marked as a duplicate of this bug. ***
*** Bug 140486 has been marked as a duplicate of this bug. ***
This bug needs to be at least critical if it's causing crashes and hangs.
(Should it be a blocker?)
Severity: major → critical
Keywords: hang, nsCatFood
*** Bug 140946 has been marked as a duplicate of this bug. ***
*** Bug 140935 has been marked as a duplicate of this bug. ***
*** Bug 139548 has been marked as a duplicate of this bug. ***
Dave, have you had a chance to look at this yet? Can you post a status here? Thanks.
I haven't had a chance to look at this yet.
Status: NEW → ASSIGNED
Attached patch Patch to fix bug — — Splinter Review 
Just don't show scrollbars when you have less than three rows.
Comment on attachment 81748 [details] [diff] [review]
Patch to fix bug

does this value depend on the specific scrollbars? (different themes have
different scrollbar thicknesses, don't they?)
A scrollbar would have to be insanely big (or row sizes insanely small) in 
order for this check to not work.  I'm comfortable with this patch.  I'm also 
comfortable with the 40px min-height patch, although I like doing it in C++ 
a little better, because mine will scale up if the whole widget got bigger.

The problem is that in order to supply a "perfect" fix, you need to be able to 
measure the scrollbar even when it is collapsed..  

Regarding what Neil suggested.  If you collapsed in the direction of the 
box while leaving the other dimension alone, you could still measure the 
height of the scrollbar, although that in and of itself would slow the 
chrome down, since you'd be doing measurements that are essentially 
irrelevant most of the time (and loading images that you may never need 
to display).

r/sr=attinasi, if you need it, for Hyatt's patch
Whiteboard: [adt1][m5+] → [adt1][m5+] [driver:asa]
This happens in mail to me when I've got 9 or 10 rows in the tree widget.  This
probably isn't going to fix the bug in that case.
I get the same thing as blizzard.  Here's what happens to me.

1.  Start Mail.  The thread pane has whatever the default # of rows is.  
2.  Shutdown
3.  Restart.  I launch mail and the cpu goes to 100%
4.  Remove my localstore.rdf and restart and everything is fine.

In step #1, the # of rows is probably somewhere between 7-10 and large enough to
show a scrollbar.
Whiteboard: [adt1][m5+] [driver:asa] → [adt1][m5+] [driver:asa] [ETA 05/02]
Hey Dave, I don't think that patch is going to solve most of the scenarios that
get us into this loop. Cavin and I have a debug windows build which we can
trigger this problem on up on the third floor. If you want to use one of our
machines to track it down you are more than welcome!

As Scott/Blizzard said, the number of rows doesn't seem to matter as far as
triggering this hang. If you have just enough rows to trigger a scrollbar in
your outliner object, then you can get into this hang. While reflowing the
scroll bar, I think we are getting another event that causes us to think we can
remove the scroll bar so we reflow to remove it but then we decide no, we really
need a scroll bar so we reflow again to paint it, and the loop repeats. Some
folks actually see the thread pane splitter move up and down as the scroll bars
are added and removed over and over again.
Just to be sure, I've applied this patch, and it doesn't fix the problem for the
thread pane scenario as described by mscott.
David, in comment 80 you say that extra measurement would be invloved. Isn't the
additional height of a scrollbar a fixed quantity ? Couldn't you just measure it
once and assume it stays the same for the lifetime of mozilla ?

I saw a lot of work going on for this bug earlier today.  Any status updates you
can give?
We understand what's happening, but not why it's happening.  This bug is very 
similar to the infamous infinite reflow problem that plagued the old tree 
widget.  We never solved that one either.  We just switched to outliners in 
the hopes that we'd never see anything like it again.  :(

Does the simple testcase in bug 139602 offer an simpler way to debug this?
Comment on attachment 81748 [details] [diff] [review]
Patch to fix bug

Hyatt's patch should fix that case. But what I am worried about is if the
number of visible rows changes so that the scrollbar should be shown again. If
the logic is going to change perhaps a better way would be like this:
PRBool oldVerticalOverflow = mVerticalOverflow;
// Don't show scrollbars if they won't fit
mVerticalOverflow = rowCount > mPageCount && mPageCount > 2;
if (mVerticalOverflow != oldVerticalOverflow) {
  ...
}
Attached file testcase — 
This testcase demonstrates the problem where uncollapsing of the scrollbar
causes change of box heights.
This testcase uses only the box, splitter and scrollbar tags. So I think this
is more likely a box layout issue.
After a splitter move, box1 and box2 get the "height" attribute set and then it
works ok.
As I said before, this change of box height causes the infinite loop.
Thats my 2 cents.
*** Bug 141824 has been marked as a duplicate of this bug. ***
*** Bug 141828 has been marked as a duplicate of this bug. ***
*** Bug 141846 has been marked as a duplicate of this bug. ***
Comment on attachment 81748 [details] [diff] [review]
Patch to fix bug

lose the unnecessary whitespace change and r=bryner
Attachment #81748 - Flags: review+
Thank you for that scrollbar/splitter testcase!!!

It turns out that the parent of the vbox and scrollbar has a different pref size
depending on whether or not the scrollbar is collapsed or not. The pref size is
calculated by the call to PopulateBoxSizes() in nsSprocketLayout::Layout().

PopulateBoxSizes() runs through all the children of a box, and calculates the
min, max, and preferred size of each child. The prefered size of the parent then
becomes the max pref size of it's children, I'm assuming this is the case when
the parent is not given a specific size.

In the case where the scrollbar is collapsed, the pref size for the parent is
270 TWIPS (the pref size of the vbox). When the scrollbar is uncollapsed, the
parent's pref size is 600 (the pref size of the scrollbar -> topScrollButton:240
+ thumb:120 + bottomScrollButton:240).

This impacts the final size of the parent because of the way the parent's size
is calculated in ComputeChildSizes():

  computedBoxSizes->size = pref + flex*sizeRemaining/spacerConstantsRemaining;

I'm not exactly sure why we add the pref size to the flex allotment calculation.

But if hyatt and mscott remember from our debugging of the mail case yesterday,
the magic number 330, which was the difference between the 2 sizes of the tree
widget we were seeing, is exactly the difference between the 2 pref sizes above
(600 - 270 = 330).

I should note that mscott and I did an experiment with removing |pref| from the
2 calculations in ComputeChildSizes(). This actually had the effect of keeping
the boxes on both sides of the splitter equal in height, which they should be,
but some gaps were appearing on the right side of the screen and at the bottom,
which leads me to believe that the pref size is actually being added/subtracted
in some other calculation somewhere else during layout.
*** Bug 141941 has been marked as a duplicate of this bug. ***
Yeah, sorry for not communicating earlier, but I'd figured this out a little 
while ago.  :)

It is correct that the pref size comes into play.  The box algorithm only 
flexes after objects are given their preferred sizes.  The bug here is that a 
tree's scrollbar should be hacked (just as a tree's body is) to have a 
preferred height of 0.

Basically we hacked the tree body not to size intrinsically, i.e., we gave it 
a pref height of 0, but we didn't do the same for the scrollbar that 
accompanies the body, which means that it defeats this hack when shown and 
suddenly gives the tree an intrinsic height equal to the minimum height of a 
scrollbar.

The reason you only see this with the "splitter" case is that the pref size 
only ends up mattering if you share your flexibility with another object.  If 
you're the only flexible object you'll obviously get the same amount of space 
regardless of your intrinsic size, but once you have to split the flexing 
space with other objects, your intrinsic size comes into play.

I have a patch forthcoming.

Comment on attachment 82173 [details] [diff] [review]
Patch to give a treebody's scrollbar a preferred height of 0, to match the preferred height of 0 we also give (in C++) for the treebody.

sr=kin@netscape.com
Attachment #82173 - Flags: superreview+
Please go ahead and check this in on the branch so it's in tomorrow's builds. 
Send any and all complainers to me.  

Awesome work Kin, MScott and Hyatt!!
Comment on attachment 82173 [details] [diff] [review]
Patch to give a treebody's scrollbar a preferred height of 0, to match the preferred height of 0 we also give (in C++) for the treebody.

r=ben@netscape.com
Attachment #82173 - Flags: review+
The fix has been checked in on the trunk and branch.

I do not have the stones to mark this fixed.  Will await testing to confirm. :)

adt1.0.0+ (on ADT's behalf) approval for checkin to the 1.0 branch. Since, this
is already on the branch, I am adding the fixed1.0.0 keyword.
Keywords: adt1.0.0+, approval, fixed1.0.0
Whiteboard: [adt1][m5+] [driver:asa] [ETA 05/02] → [adt1][m5+] [driver:asa] [ETA 05/03] [Needs a=]
*** Bug 141960 has been marked as a duplicate of this bug. ***
Should all scrollbars have a preferred height and width of 0?
I assume your question is, "Should all scrollbars have a preferred size of 0 
along their axis of orientation?"  In other words, should all horizontal 
scrollbars have a pref width of 0, and all vertical scrollbars have a pref 
height of 0?

I think the answer is yes, actually.  To minimize risk, I say stick with this 
for the branch, but we should probably open a new bug to do this for all 
scrollbars in CSS.

*** Bug 141966 has been marked as a duplicate of this bug. ***
Hmmm, so I added the height="0" patch to the testcase in this bug, but it 
doesn't fix the problem, so I don't think it will work for the tree's scrollbar 
either.

The problem is that the box code that gets the pref, min, and max size, will 
fall back to using the children's pref, min, and max size if *both* the width 
and height versions of those properties are not defined.

Setting both a width="0" and height="0" doesn't work on the <scrollbar> because 
the children min-width and min-height kicks in. The only way I could get the 
desired effect was to do this in the testcase:

  <scrollbar style="height: 0px; min-width: 16px; min-height: 16px;"
   id="scrollbar" orient="vertical"/>

or

  <scrollbar height="0" minwidth="16" minheight="16"
   id="scrollbar" orient="vertical"/>

We obviously can't hard code the 16px, taken from the scrollbar skin css files, 
so perhaps we need to use defaults like these in the skin css files? Or hack the 
tree code to get these values and set them on it's scrollbar in C++?

Note, you can't use a min value less then 16 (at least on windows) or the 
scrollbars start to look squashed.
Note: You can substitute min-height:1px and minheight="0" in my <scrollbar> 
examples above and it will still work. I haven't bothered to look at why using 
0px in the style version doesn't act the same as the minheight="0" attribute.

As an experiment, I added the min attributes above to tree.xml, and it prevents 
the infinite loop in the address book.
Adding height="0" did fix the testcase for me, but I'm on Windows XP, and the 
theme stuff is specifying a min-height of 0.  Windows XP could also even deal 
with the shrinking scrollbars.  Darn.  Switching to Modern, so I can play with 
this some more.

ok, |height="0" minwidth="16" minheight="0"| fixed it in all places where I was
able to reproduce it.
I see there only one cosmetic problem, when you resize thread pane to 1 or 2 rows
scrollbar buttons overlap tree widget.
It might be another bug in painting code, which doesn't check this possible
ovelapping while painting a background image (in this case - scrollbar button)
But that's only minor.

When used in conjunction with the already checked in patch, I think this nails
it.  This also makes the thumb shrink down to 0 size in modern, and then the
buttons even scale down (making for a much better scrollbar experience at small
sizes).

This has also been tested with the WinXP native theme stuff and found to work
ok.  It needs to be tested on Win2k and then on Linux and Mac with their native
theme renderers.

I'd like someone to try this patch and see if it fixes the tree widget case in
mail.  In theory it should, because the minimum size of a scrollbar should be
computed as 0 now.
Unfortunately Cavin and the laptop he reproduces this on is at home today
without a  car. So we've lost our good stable test case of the infinite loop in
mail to try this out on. I'll see if I can find someone else who can reproduce
this at will that can try this latest patch. 
sorry, but this patch only fixes the testcase, I can reproduce the infinite loop
in DOM inspector - XBL bindings view and in Addressbook even.

removing the fixed1.0.0 keyword because we don't have something checked in that
we know fixes it yet. But it sounds like we are getting close.
Keywords: fixed1.0.0
With this patch, I ocassionally see a gap between the slider and the bottom 
scrollbutton. I noticed this because I put a different color on each of the 
boxes in the testcase so I can see where each one begins/ends. This gap is not 
there without the patch. The gap will come and go, if you drag the splitter up 
and down, or resize the window vertically.

FYI, if I augment hyatt's original height="0" patch (attachment 82173 [details] [diff] [review]) so that 
the tree's <scrollbar> has |height="0" minwidth="0" minheight="0"| it still 
prevents the hang in the addressbook.

It turns out I only had to use a minwidth="16" because at the time I was 
specifying width="0". Leaving off the width="0" removes this necessity.
hyatt's patch works great on linux - modern skin
but on linux and with classic skin, scrollbar buttons don't scale, because
they have already defined min-width and min-height in CSS

scrollbars.css
                                                                               
           scrollbarbutton {
  background: -moz-Dialog no-repeat 0px 1px;
  min-width: 16px;
  min-height: 16px;
}

I think the same you should see on windows other than XP - classic skin
since linux classic and windows classic share the same files.
Attached patch Branch only fix — — Splinter Review 
This a patch that takes hyatt's initial height: 0px change and adds a min and
max height of zero as well for the tree. 

I have confirmed reports from cavin that this fixes the hang. That's the first
patch to actually get rid of it. 

I would like to get this patch r/sr'ed and into the branch. 

We can continue the deeper fix on the trunk which is playing aoround with the
scrollbar elements. 

Objections? Thoughts? reviews? =)
I should add that the other patches don't actually fix the mail hang for us
(although Jan says they did). We still saw the pinging of the CPU with the
latest patches in this bug. 

For those of testing on release builds, you can probably make this change to
tree.xml yourself. It should be in a file called global.jar. You can open it up
using winzip or some other application, make the same change in the "Branch only
fix" patch on that file. 
*** Bug 126328 has been marked as a duplicate of this bug. ***
I tried out the Branch only fix and that seemed to fix the problem ( windows 2k).
Yes, the patch to tree.xml fixes the mail hang for me on my machine too.
Yeah, that'll work.  I hate skins.

Comment on attachment 82237 [details] [diff] [review]
Prevent the minsizes of the buttons and thumb from coming into play by giving them all minwidths/heights of 0.

This patch is worth it just for its general niceness :-)
Comment on attachment 82269 [details] [diff] [review]
Branch only fix

This patch came from Kin and has a
r=mscott
sr=hyatt

Checking into the trunk now, then I'll email for drivers approval.
Attachment #82269 - Flags: superreview+
Attachment #82269 - Flags: review+
Comment on attachment 82269 [details] [diff] [review]
Branch only fix

a=asa (on behalf of drivers) for checkin to the 1.0 branch
Attachment #82269 - Flags: approval+
Ok, I checked in the patch on the branch and the trunk. 

Let's mark this fixed and spin off further patches for the trunk (like the
minxize on the scrollbar) into a new bug.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 23 years ago
Keywords: fixed1.0.0
Resolution: --- → FIXED
I just downloaded the 5/4 Branch build and I no longer hang every time I start
up mail.  Thanks for getting this one fixed.
*** Bug 142279 has been marked as a duplicate of this bug. ***
*** Bug 142696 has been marked as a duplicate of this bug. ***
*** Bug 142882 has been marked as a duplicate of this bug. ***
With trunk build 2002050910 on Linux I am still seeing an infinite reflow in
mailnews. I am using a maximized mailnews window on a 800x600 screen. When I
pick "View->Headers->All" and then view any message with attachments, The
attachment pane reflows endlessly.
Comment on attachment 82237 [details] [diff] [review]
Prevent the minsizes of the buttons and thumb from coming into play by giving them all minwidths/heights of 0.

according to kin, this needs work.
kin writes:  "this fix has some side effects like leaving gaps in some cases
between the scrollbar elelments"
Attachment #82237 - Flags: needs-work+
Kin, this fixes scrollbars for me, based on Hyatt's patch but with extra css.
Attached file Test case — 

    
    

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

    
    

    
  


  
From bug# 91662 

I just noticed that I could drag the vertical spliter bar (the one that 
sseparate the upper 2 listbox) to the left till it CROSSED the scrollbar of the

folder list listbox momentarily. 

You need to use 3-window mode, and maybe large fonts in Window$' scheme, 
800x600 resolution. 

This behaviour may be relatd to this bug, but I am not sure. I will attach an 
image...

    
    

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

    
    

    
  
v
Status: RESOLVED → VERIFIED

    
    

    
  
I still see this problem :
OS : win XP SP1
mozilla 1.4
screen size 800x600

When the mail window is not maximized, but only about 80% of the screen size and
the selected folder is "Sent" and the amount of message in it is just about the
amount that fits the vertical size of the message list window, then  mozilla
goes into a flashing loop, where it draw a vertical scrollbar for the message
list window, then deletes the scrollbar and repeat.

If this is fixed after v1.4 then disregard my message, otherwise please reopen
this bug.

    
    

    
  
Oh, I forgot :

If I maximize the Mail window, the flashing stops. If I go back to non-maximized
size, it starts again. It also does not stop if I resize the the window by a
small amount. After I moved the line that separates the message list from
message body display ( in other words I resized the message list panel ) the
flashing stopped and I could not start it again, that is why I did not even try
v1.5 as it is hard to reproduce.
Also while flashing, if I selected another message group ( with zero messages )
in the tree on the left, then the flashing stopped. When I selected back the
"Sent" folder , it started to flash again ( this was before fixing it by
resizing the message list panel )


    
    

    
  
I have seen this problem several times in Mozilla 1.4 under Windows 2000 (I have
not tested on any other platform). 

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: