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: