Closed Bug 46043 Opened 25 years ago Closed 25 years ago

[FLOAT] (Adds on?) site crashes

Categories

(Core :: Layout, defect, P1)

x86
All
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: devotip, Assigned: waterson)

References

()

Details

(Keywords: crash, testcase, Whiteboard: [nsbeta3+] FIX IN HAND)

Attachments

(5 files)

From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; m17) Gecko/20000720 BuildID: 2000072008 open the url the browser hangs forever, no page refresh at all but is not crashing. Reproducible: Always Steps to Reproduce: 1.open the url http://www.radio-stations.net/language/esperanto.htm 2.Wait to have the page loaded 3. Actual Results: The browser is dead Expected Results: page loaded
Accepting, adding crash keyword (because it crashed on me) and making test
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash, makingtest
Summary: The browser hangs → The browser crashes
I can no longer reproduce the crash on thing page after the first crash. I am guessing that the crash (which I confirmed) and the reported crash have to do with the add-banner, and hense is practicly impossible to testcase.
Keywords: makingtest
Summary: The browser crashes → (Adds on?) site crashes
After removing the banners the browser is still crashing
I dont get any crashes, but I do see the deadlock before the page is loaded. I have a small testcase for that (maybe it will crash for others.) I am using build 2000-07-20-08 on Windows 98 SE.
Severity: normal → critical
Keywords: testcase
I still do not see any crash or deadlock on the original page, or the first test case. However I do see a deadlock on the second test case, but only at 800x600 resolution, not at a higher resolution. It seems like to me that the 3 <IFRAME>'s get layed out like this: <IFRAME><IFRAME> <IFRAME> and that for some reason the <ol> gets layed out to the LEFT of this, which is off the page and crashes. This would explain why at higher resolutions it does not crash (there's enough space to put the <ol> to the left)
so, this is a iframe issue?
over to HTML Element
Assignee: asa → clayton
Component: Browser-General → HTML Element
QA Contact: doronr → petersen
win98 build 2000072108 I'm getting the effect I described with attachment I provided at 1280x1024 16 bit color resolution. Actually the browser hangs, it's not a crash now.
Triaging clayton's list: ------------------------ I do see a crash ( loading 2nd attachment )!! Here is the trace. nsDebug::Abort(const char * 0x020b2320, int 4137) line 315 + 3 bytes nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & {...}, nsLineBox * 0x02df4950, int * 0x0012c3b8, int 0, int 0) line 4137 + 16 bytes nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x02df4950, int * 0x0012c3b8, int 0) line 3262 + 29 bytes nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2951 + 27 bytes nsBlockFrame::Reflow(nsBlockFrame * const 0x02df48c8, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 1756 + 15 bytes nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState & {...}, nsReflowReason eReflowReason_Initial, nsIFrame * 0x02df48c8, const nsRect & {...}, int 0, int 0, int 1, nsMargin & {...}, unsigned int & 0) line 519 + 36 bytes nsBlockReflowContext::ReflowBlock(nsIFrame * 0x02df48c8, const nsRect & {...}, int 0, int 0, int 1, nsMargin & {...}, unsigned int & 0) line 344 + 50 bytes nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineBox * 0x02df4a74, int * 0x0012cf10) line 3880 + 56 bytes nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x02df4a74, int * 0x0012cf10, int 0) line 3144 + 23 bytes nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2951 + 27 bytes nsBlockFrame::Reflow(nsBlockFrame * const 0x02df4840, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 1756 + 15 bytes nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState & {...}, nsReflowReason eReflowReason_Initial, nsIFrame * 0x02df4840, const nsRect & {...}, int 1, int 0, int 1, nsMargin & {...}, unsigned int & 0) line 519 + 36 bytes nsBlockReflowContext::ReflowBlock(nsIFrame * 0x02df4840, const nsRect & {...}, int 1, int 0, int 1, nsMargin & {...}, unsigned int & 0) line 344 + 50 bytes nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineBox * 0x02df4b28, int * 0x0012da68) line 3880 + 56 bytes nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x02df4b28, int * 0x0012da68, int 1) line 3144 + 23 bytes nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2951 + 27 bytes nsBlockFrame::Reflow(nsBlockFrame * const 0x02e7cb2c, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 1756 + 15 bytes nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState & {...}, nsReflowReason eReflowReason_Incremental, nsIFrame * 0x02e7cb2c, const nsRect & {...}, int 1, int 0, int 1, nsMargin & {...}, unsigned int & 0) line 519 + 36 bytes nsBlockReflowContext::ReflowBlock(nsIFrame * 0x02e7cb2c, const nsRect & {...}, int 1, int 0, int 1, nsMargin & {...}, unsigned int & 0) line 344 + 50 bytes nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineBox * 0x02e7cb78, int * 0x0012e5c0) line 3880 + 56 bytes nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x02e7cb78, int * 0x0012e5c0, int 1) line 3144 + 23 bytes nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2951 + 27 bytes nsBlockFrame::Reflow(nsBlockFrame * const 0x02e7cae0, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 1756 + 15 bytes nsContainerFrame::ReflowChild(nsIFrame * 0x02e7cae0, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0, unsigned int 0, unsigned int & 0) line 693 + 31 bytes CanvasFrame::Reflow(CanvasFrame * const 0x02e7bc6c, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 306 nsBoxToBlockAdaptor::Reflow(nsBoxLayoutState & {...}, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0, int 0, int 0, int 10800, int 10215, int 1) line 813 nsBoxToBlockAdaptor::DoLayout(nsBoxToBlockAdaptor * const 0x02e7ca78, nsBoxLayoutState & {...}) line 484 + 52 bytes nsBox::Layout(nsBox * const 0x02e7ca78, nsBoxLayoutState & {...}) line 1002 nsScrollBoxFrame::DoLayout(nsScrollBoxFrame * const 0x02e7bd84, nsBoxLayoutState & {...}) line 377 nsBox::Layout(nsBox * const 0x02e7bd84, nsBoxLayoutState & {...}) line 1002 nsContainerBox::LayoutChildAt(nsBoxLayoutState & {...}, nsIBox * 0x02e7bd84, const nsRect & {...}) line 593 + 16 bytes nsGfxScrollFrameInner::LayoutBox(nsBoxLayoutState & {...}, nsIBox * 0x02e7bd84, const nsRect & {...}) line 1003 + 17 bytes nsGfxScrollFrameInner::Layout(nsBoxLayoutState & {...}) line 1086 nsGfxScrollFrame::DoLayout(nsGfxScrollFrame * const 0x02e7bce0, nsBoxLayoutState & {...}) line 1011 + 15 bytes nsBox::Layout(nsBox * const 0x02e7bce0, nsBoxLayoutState & {...}) line 1002 nsBoxFrame::Reflow(nsBoxFrame * const 0x02e7bca8, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 728 nsGfxScrollFrame::Reflow(nsGfxScrollFrame * const 0x02e7bca8, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 719 + 25 bytes nsContainerFrame::ReflowChild(nsIFrame * 0x02e7bca8, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0, unsigned int 0, unsigned int & 0) line 693 + 31 bytes ViewportFrame::Reflow(ViewportFrame * const 0x02e7bc30, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) line 546 nsHTMLReflowCommand::Dispatch(nsHTMLReflowCommand * const 0x03c17330, nsIPresContext * 0x03b63f00, nsHTMLReflowMetrics & {...}, const nsSize & {...}, nsIRenderingContext & {...}) line 145 PresShell::ProcessReflowCommands(int 0) line 4209 PresShell::FlushPendingNotifications(PresShell * const 0x03ba61b0) line 3308 PresShell::DidCauseReflow() line 4159 PresShell::ContentAppended(PresShell * const 0x03ba61b8, nsIDocument * 0x0445b8f0, nsIContent * 0x03ba2888, int 0) line 3411 nsDocument::ContentAppended(nsDocument * const 0x0445b8f0, nsIContent * 0x03ba2888, int 0) line 1805 Giving bug to waterson and ccing buster and nisheeth.
Assignee: clayton → waterson
Setting OS to All. I can reproduce this crash (abort) with a PC/Linux debug build from 7/21: Steps to reproduce: 1.) Load the original URL or the first attachment 2 [details] [diff] [review].) Resize the browser window to make it larger Alternative steps to reproduce: 1.) Load the second attachment 2 [details] [diff] [review].) Resize the browser window to make it very small 3.) Resize the browser window to make it very large The last shell output before the "###!!! Abort:..." is: > Block(li)(1)@0x4250acf8: yikes! spinning on a line over 1000 times!
OS: Windows 98 → All
crasher
Status: NEW → ASSIGNED
Keywords: nsbeta3
Priority: P3 → P1
Whiteboard: [nsbeta3+]
Target Milestone: --- → M18
Attached file even simpler test case
The simplified test case illustrates that this bug is a problem with floaters; adding grafitti to indicate this. (To repro with that test case, use viewer, or shrink your browser window down to l.t. 620px width.) N.B. that if we reverse the order of the <div>'s (so that the 300px div comes first), we'll just flat out ignore the 600px float with respect to available space computation. Also, dbaron notes my inline CSS is invalid. Need to add 'px' if its to be accepted in other than quirks mode.
Summary: (Adds on?) site crashes → [FLOAT] (Adds on?) site crashes
Attached patch fixSplinter Review
I am too stupid at this hour to understand what troy was thinking when he put the "aBand->mLeft < rightEdge" test in here. I can't see how this would've ever worked, so I took it out, and it seems to fix the bug.
Whiteboard: [nsbeta3+] → [nsbeta3+] FIX IN HAND
The code in question is: while ((aBand->mTop == topOfBand) && (aBand->mLeft < rightEdge)) { It looks to me that this is saying, "do the loop for all rects whose top edge is the top of the original rect (topOfBand) AND whose left edge is inside the requested rect", or graphically: . +---+-+----.-+-+-------+ <-- topOfBand | 1 | | . | | | +---| | 2 . | | 3 | | | . | +-------+ + +----.-+ . . <-- rightEdge In this example, rects 1 and 2 would be considered, because they share the topEdge and their left boundary is to the left of rightEdge. Rect3 is not considered because it's beyond the bounds of the requested area. The dead spaces between the numbered rects are meant to represent available space. This seems reasonable. Looking at the test case and the proposed patch, I can't figure out why your change makes it work. I think this needs some more careful analysis. It's a pretty important function, and I can't convince myself the fix won't introduce regressions in pages with interesting floaters. Chris, let me know if you want me to work with you on this, or if you're just going to keep going on it.
Thanks for the explanation. I'll look at the code some more, maybe we can talk later today.
Attached patch better fixSplinter Review
I think this is the right fix. The problem was that the block band data be advanced to a new line that had no available space, according to the space manager, but would fail to clear the mLeftFloaters and mRightFloaters variables. This would cause the frame think that it was "impacted by floaters", so we'd miss the special case in CanPlaceFrame() when it's "not safe to break". This would cause us to continually try to re-break the frame over and over again. buster: whatdya think?
too sleepy to grok this tonight. I'll look into it tomorrow am.
r=buster I haven't actually tested it, but reading the code it looks like the right thing to do. Good work.
checked in.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Marking verified in the the Aug 30th build.
Status: RESOLVED → VERIFIED
SPAM. HTML Element component deprecated, changing component to Layout. See bug 88132 for details.
Component: HTML Element → Layout
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: