Closed
Bug 128809
Opened 23 years ago
Closed 22 years ago
Mail layout goes into infinite loop when "view headers all" is activated
Categories
(SeaMonkey :: MailNews: Message Display, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.1alpha
People
(Reporter: carljohan, Assigned: janv)
References
Details
(Keywords: hang, polish, Whiteboard: [adt2 RTM],custrtm- [Needs Reviews] [ETA Needed])
Attachments
(7 files, 6 obsolete files)
2.81 KB,
text/plain
|
Details | |
48.38 KB,
image/png
|
Details | |
47.81 KB,
image/png
|
Details | |
60.60 KB,
image/jpeg
|
Details | |
4.20 KB,
patch
|
janv
:
review+
|
Details | Diff | Splinter Review |
1.50 KB,
patch
|
Details | Diff | Splinter Review | |
6.78 KB,
patch
|
kinmoz
:
review+
bryner
:
superreview+
scc
:
approval+
|
Details | Diff | Splinter Review |
I was viewing my mail with "View Headers All" activated. I encountered some email from a mailinglist with a huge amount of headers. When Mozilla tried to display the mail the layout of the mail entered an infinite loop and started flashing at the bottom ov the mail. I can repeat this with several mails in my inbox, all with huge amounts of headers. The headers are so big that mozilla can't display the mailbox, the headers and the mail body at the same time without squeezing the mailbox down to just two entries. Steps to reproduce: View the mail I'm going to attach with "View Headers All" activated. Expected Result: Mozilla should display the mail. Actual Result: Mozilla enters an infinite layout loop. Mozilla is still responsive but takes a lot of CPU. Viewing another mail/mailbox makes it alright again.
This mail is one of several mail with large amounts of headers that forces mozilla into an infinite layout loop.
Look at the lower part of the mail. The layout is a bit broken but in a tenth of a second it changes its' layout to the corect one that is shown in the second attachment.
This is the correct layout which is displayed for some tenths of a second and then the cycle repeats.
The build I'm using is Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.8) Gecko/20020212 And I can reproduce it every time.
Comment 5•23 years ago
|
||
please change mime type of image attachments to image/png
Sorry. I must have been sleeping when I uploaded the first pictures as text/plain. Someone who is authorized please obsolete the first two pictures.
Comment 9•22 years ago
|
||
Reproduced with 2002040211/Linux. Another mail file that causes an infinite loop: http://bugzilla.mozilla.gr.jp/showattachment.cgi?attach_id=648
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•22 years ago
|
Attachment #72384 -
Attachment is obsolete: true
Updated•22 years ago
|
Attachment #72385 -
Attachment is obsolete: true
Reporter | ||
Comment 10•22 years ago
|
||
The Japaneese mail sends my Mozilla into an infinite loop as well, but only if I make the mailwindow smaller first. The problem occurs when there is so many headers so that the listing of mails, the headers and the mail itself can fit in one window.
Comment 11•22 years ago
|
||
accepting. yulian just found this on win2k, and laurel can see it on win98, so marking all. so trick for reproducing it: 1) open a folder with 4 messages 2) I found it easier to reproduce with a message with no body. 3) view headers all 4) play with the size of the whole window, get small enough, and you'll probably hit the freeze note, yulian tried in build that contained that "no height set" fix that mscott, hyatt and kin came up with. note, view all headers is off by default I'll try to get some stack traces.
Severity: major → critical
Status: NEW → ASSIGNED
Keywords: hang
OS: Linux → All
Whiteboard: nsbeta1
Target Milestone: --- → mozilla1.1alpha
Updated•22 years ago
|
Whiteboard: nsbeta1
Updated•22 years ago
|
Comment 12•22 years ago
|
||
see also bug http://bugzilla.mozilla.org/show_bug.cgi?id=121583 "work around the infinite reflow loop in various windows that use tree widgets by forcing a min height and min width on the the tree. This fixes the famous 100 % CPU hang" that fix has only landed on the 1.0 branch and the trunk, not the mach v beta branch.
Comment 13•22 years ago
|
||
unless of course it landed before we branched for mach v beta. I'll check...
Comment 14•22 years ago
|
||
yes, it must have landed before the mach v branch branched from the 1.0 branch. Maybe a similar workaround can fix this problem.
Comment 15•22 years ago
|
||
met with kin today, he's going to do some investigation that will hopefully yield to a "real" fix in layout. on my end, I'm going to try to come up with something less risky, but more of a hack to our xul (or .css or .xml), like they did for #121583
Comment 16•22 years ago
|
||
It looks like this bug is a variation on bug #121583, except it's not the scrollbar in the tree that's changing the size of the tree body frame. It looks like in this particular case, the box layout code is doing a multi-pass-layout at several points in the box tree. One of the first passes makes the size of the tree big enough that it thinks it doesn't need a scrollbar, so it posts an underflow DOMEvent, and then during a subsequent pass, it resizes the tree so that it does need a scrollbar, so it posts an overflow DOMEvent. When reflow is done, processing the 1st dom event, collapses the scrollbar and posts a reflow event. The 2nd DOMEvent simply un-collapses the scrollbar so that when we process the posted reflow event, we are back to square one and the cycle continues. I'm thinking we need to break this cycle by either: 1. Removing the DOMEvent posting mechanism to hide/show the scrollbars, and do it some other way. or: 2. Add some mechanism that allows us to track and revoke previously posted events when we're about to post one that supercedes it. I'm leaning towards trying to find a way to do #1.
Comment 17•22 years ago
|
||
kmcclusk says hewitt@netscape.com is handling box bugs while hyatt is away. hewitt, can you take a look at this one?
Comment 18•22 years ago
|
||
*** Bug 145252 has been marked as a duplicate of this bug. ***
Comment 19•22 years ago
|
||
neil's patch (http://bugzilla.mozilla.org/attachment.cgi?id=84232&action=view) from http://bugzilla.mozilla.org/show_bug.cgi?id=121583#c136 seems to fix this. I'll confirm.
Comment 20•22 years ago
|
||
>neil's patch (http://bugzilla.mozilla.org/attachment.cgi?id=84232&action=view)
>from http://bugzilla.mozilla.org/show_bug.cgi?id=121583#c136 seems to fix this.
>I'll confirm.
false alarm. it doesn't fix it.
Comment 21•22 years ago
|
||
May I suggest a fix for this that would solve this and many more problems? Making the "headers" a separate resizable frame with a scrollbar, like the other frames in the window, just read-only: With most bigger headers the current solution leaves very little room for the other frames - folder list, folders and mail contents, swallowing most of the available area. It would be very neat if the headers were selectable (it's no fun retyping "references" or such...) and one could decide how much screen real estate they want to spend on the headers.
Comment 22•22 years ago
|
||
This patch fixes some issues I was having displaying attachment 72383 [details] in the standalone message window. Note: I also used my attachment to bug 121583.
Comment 23•22 years ago
|
||
*** Bug 147534 has been marked as a duplicate of this bug. ***
Comment 24•22 years ago
|
||
note: I've tried out all neils patches together, and I'm still getting the hang.
Comment 25•22 years ago
|
||
*** Bug 147767 has been marked as a duplicate of this bug. ***
Comment 26•22 years ago
|
||
Oops, patch only fixes the message window, not the 3panes :-( But just to make sure that I'm on the right lines, what happens if you: 1. Collapse the message pane 2. Select the dodgy message 3. Uncollapse the message pane I don't see any problems in this case.
Comment 27•22 years ago
|
||
HideAccountCentral() removes these collapsed attributes.
Comment 28•22 years ago
|
||
*** Bug 148392 has been marked as a duplicate of this bug. ***
Comment 29•22 years ago
|
||
*** Bug 140969 has been marked as a duplicate of this bug. ***
Comment 30•22 years ago
|
||
I tried out all neil's patches, including the new one to messagepane.xul, no luck. > Oops, patch only fixes the message window, not the 3panes :-( > But just to make sure that I'm on the right lines, what happens if you: > 1. Collapse the message pane > 2. Select the dodgy message > 3. Uncollapse the message pane with the "evil" message, and with the right 3 pane window size, that can cause the infinite loop that kin describes in http://bugzilla.mozilla.org/show_bug.cgi?id=128809#c16 (Note, the "evil" message I'm using is http://bugzilla.mozilla.org/attachment.cgi?id=72383&action=view, but I've seen it with a variety of other message, when in view all headers mode.) to reproduce this problem: 1) put the evil message in a folder with no other messages. 2) go into view all header mode 3) grab the bottom of the 3 pane window and move upwards. at a certain point, that seems to make the thread pane just small enough where we go into the infinite loop. hewitt and I are talking over aim about some things we can do to the underflow event handler in tree.xml hewitt's idea: you're going to allow us to see the overflow state (nsTreeBoxFrame's mVerticalOverlow) as a property on nsITreeBoxObject, and then when handling underflow, we would check that, and not collapse the toolbar in that scenario.
Comment 31•22 years ago
|
||
Comment 32•22 years ago
|
||
that last hack depends on shaver's fix for http://bugzilla.mozilla.org/show_bug.cgi?id=146210, as it uses Date.now() basically, I'm cheating and deciding that if we get an underflow event followed by an overflow event, followed by an underflow event within 1 second, to ignore the second underflow event, and not collapse the scrollbars. this prevents the infinite loop, but as you can guess, if the loop is on a slow machine, 1 second might not be enough. as you can also guess, we are dropping underflow events on the floor, which can lead to scenarios where we show the scroll bar, when we shouldn't. see the upcoming screen shot. hewitt's idea sounds much better, but this is just an alternative patch, in the spirit of kin's suggestion from http://bugzilla.mozilla.org/show_bug.cgi? id=128809#c16 "2. Add some mechanism that allows us to track and revoke previously posted events when we're about to post one that supercedes it."
Comment 33•22 years ago
|
||
Comment 34•22 years ago
|
||
re-assign to shuehan. shuehan see http://bugzilla.mozilla.org/show_bug.cgi?id=121583 to see how a similar bug was fixed.
Assignee: sspitzer → shliang
Status: ASSIGNED → NEW
Comment 35•22 years ago
|
||
*** Bug 150573 has been marked as a duplicate of this bug. ***
Comment 36•22 years ago
|
||
> at a certain point, that seems to make the thread pane just small enough where
> we go into the infinite loop.
D'oh! Perhaps this doesn't apply to the alternate 3-pane layout? There's a limit
to how small I can make the thread pane in that layout...
Comment 37•22 years ago
|
||
*** Bug 123882 has been marked as a duplicate of this bug. ***
Comment 38•22 years ago
|
||
Shuehan, what's the eta on the fix for this?
Comment 39•22 years ago
|
||
Shuehan, should this be reassigned to Joe?
Comment 40•22 years ago
|
||
reassigning to joe since he has said that he wanted to work on it.
Assignee: shliang → hewitt
Comment 41•22 years ago
|
||
This fix plugs the infinite loop by checking the overflow state of the tree before collapsing/hiding the scrollbar, and only doing so if it has changed.
Updated•22 years ago
|
Attachment #85063 -
Attachment is obsolete: true
Attachment #85775 -
Attachment is obsolete: true
Attachment #86879 -
Attachment is obsolete: true
Updated•22 years ago
|
Whiteboard: [adt1 RTM],custrtm- → [adt1 RTM],custrtm- [Needs Reviews] [ETA Needed]
Assignee | ||
Comment 42•22 years ago
|
||
kin, your analysis gave me an idea I think once I saw, box code calling SetBounds() 2 times in some layout function. We call CheckVerticalOverflow() in nsTreeBodyFrame::SetBounds() and CheckVerticalOverflow() posts underflow or overflow events when needed. I think it would be better to call CheckVerticalScrollbar() after reflow completed its work - maybe in DidReflow() or there is a reflow callback mechanism which hyatt added a long time ago to fix problems with scrollbar in the old tree implementation. If we don't figure out a real fix, we will have to stick with the hack.
Assignee | ||
Comment 43•22 years ago
|
||
ok, I added DidReflow() to nsTreeBodyFrame, but it is never called. I checked nsLeafBoxFrame - the same (nsTreeBodyFrame inherits from nsLeafBoxFrame) It seems that this stopped working recently, since I'm sure that it worked some time ago.
Comment 44•22 years ago
|
||
*** Bug 152302 has been marked as a duplicate of this bug. ***
Comment 45•22 years ago
|
||
lowering impact to adt2 rtm.
Whiteboard: [adt1 RTM],custrtm- [Needs Reviews] [ETA Needed] → [adt2 RTM],custrtm- [Needs Reviews] [ETA Needed]
Comment 46•22 years ago
|
||
With a standard Mozilla build, loading the message causes it to take over all available space in the window. With all my 30+ patches that I have written and installed on my PC for my day-to-day use the message loads in the space that the splitter has allocated to it thus completely avoiding this problem. As you have noticed I have not yet tracked down the patch that actually resolves the problem.
Comment 47•22 years ago
|
||
*** Bug 153626 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 48•22 years ago
|
||
This patch makes us posting only one overflow/underflow event. I didn't test it much. If it fixes the problem for someone else, please let me know. Thanks.
Assignee | ||
Comment 49•22 years ago
|
||
Comment on attachment 89249 [details] [diff] [review] other fix sorry, I just ran into infinite loop again with this patch
Attachment #89249 -
Flags: needs-work+
Comment 50•22 years ago
|
||
This attacks the problem by preventing the message from taking up too much space.
Comment 51•22 years ago
|
||
Has anyone noticed that if you use the splitter above the message header to decrease the height of the message pane, at a certain point (around 200px) the message pane browser refuses to shrink any further, and will actually overlap the status bar?
Comment 52•22 years ago
|
||
*** Bug 154021 has been marked as a duplicate of this bug. ***
Comment 53•22 years ago
|
||
Joe, are you talking about bug 41994 or bug 119629? (are those two the same?).
Assignee | ||
Comment 54•22 years ago
|
||
Joe's patch fixes this bug for me. I haven't found a better solution yet. So we can fall back with this fix until we find a better one.
Assignee | ||
Comment 55•22 years ago
|
||
Comment on attachment 87778 [details] [diff] [review] fix r=varga
Attachment #87778 -
Flags: review+
Comment 56•22 years ago
|
||
Comment on attachment 87778 [details] [diff] [review] fix >- event.preventBubble(); Why don't you want the event to bubble all of the time any more? >+ if (!this.parentNode.treeBoxObject.overflowed && this.parentNode.treeBoxObject.overflowed != this._lastOverflow) { Why not just: if (!this.parentNode.treeBoxObject.overflowed && this._lastOverflow) { >- event.preventBubble(); Same again. >+ if (this.parentNode.treeBoxObject.overflowed && this.parentNode.treeBoxObject.overflowed != this._lastOverflow) { Almost same again: if (this.parentNode.treeBoxObject.overflowed && !this._lastOverflow) {
Comment 58•22 years ago
|
||
*** Bug 157599 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 59•22 years ago
|
||
This fix uses the nsIReflowCallback mechanism to check vertical overflow only when reflow finished.
Attachment #89249 -
Attachment is obsolete: true
Comment 60•22 years ago
|
||
*** Bug 131145 has been marked as a duplicate of this bug. ***
Comment 61•22 years ago
|
||
I tried the last patch "new fix" and it sems to fix the hang. One thing I noticed is that based on size of the mail window, sometimes the thread pane is not visible. If I increase the size of the window I can see it again.
Assignee | ||
Comment 62•22 years ago
|
||
thanks!
> sometimes the thread pane is not visible
I think, this is expected behaviour, thread pane is flexible and headers take so
much space that thread pane have to shrink to minimal height.
I wonder what whould happen after adding min heigth to thread pane.
I do hope not another hang :)
Comment 63•22 years ago
|
||
Comment on attachment 91545 [details] [diff] [review] new fix sr=bryner
Attachment #91545 -
Flags: superreview+
Comment 64•22 years ago
|
||
Jan, who can review this? We're short on time for 1.1b.
Comment 65•22 years ago
|
||
Comment on attachment 91545 [details] [diff] [review] new fix r=kin@netscape.com
Attachment #91545 -
Flags: review+
Comment 66•22 years ago
|
||
> sometimes the thread pane is not visible That's bug 106679.
Comment 67•22 years ago
|
||
Comment on attachment 91545 [details] [diff] [review] new fix a=scc for checkin to the mozilla trunk
Attachment #91545 -
Flags: approval+
Assignee | ||
Comment 68•22 years ago
|
||
checked in
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 69•22 years ago
|
||
*** Bug 159608 has been marked as a duplicate of this bug. ***
Comment 70•22 years ago
|
||
OK using sep16 commercial trunk build: win98, mac OS 10.1, linux rh6.2
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•