[FLOAT] (Adds on?) site crashes

VERIFIED FIXED in M18

Status

()

P1
critical
VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: devotip, Assigned: waterson)

Tracking

({crash, testcase})

Trunk
x86
All
crash, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3+] FIX IN HAND, URL)

Attachments

(5 attachments)

(Reporter)

Description

18 years ago
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

Comment 1

18 years ago
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

Comment 2

18 years ago
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
(Reporter)

Comment 3

18 years ago
Created attachment 11679 [details]
page with removed the banners that kills the browser(win98 2000072008)
(Reporter)

Comment 4

18 years ago
After removing the banners the browser is still crashing

Comment 5

18 years ago
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

Comment 6

18 years ago
Created attachment 11684 [details]
Testcase causing deadlock

Comment 7

18 years ago
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)

Comment 8

18 years ago
so, this is a iframe issue?

Comment 9

18 years ago
over to HTML Element
Assignee: asa → clayton
Component: Browser-General → HTML Element
QA Contact: doronr → petersen
(Reporter)

Comment 10

18 years ago
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.

Comment 11

18 years ago
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
(Assignee)

Comment 13

18 years ago
crasher
Status: NEW → ASSIGNED
Keywords: nsbeta3
Priority: P3 → P1
Whiteboard: [nsbeta3+]
Target Milestone: --- → M18
(Assignee)

Comment 14

18 years ago
Created attachment 13204 [details]
even simpler test case
(Assignee)

Comment 15

18 years ago
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
(Assignee)

Comment 16

18 years ago
Created attachment 13258 [details] [diff] [review]
fix
(Assignee)

Comment 17

18 years ago
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.
(Assignee)

Updated

18 years ago
Whiteboard: [nsbeta3+] → [nsbeta3+] FIX IN HAND

Comment 18

18 years ago
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.
(Assignee)

Comment 19

18 years ago
Thanks for the explanation. I'll look at the code some more, maybe we can talk
later today.
(Assignee)

Comment 20

18 years ago
Created attachment 13405 [details] [diff] [review]
better fix
(Assignee)

Comment 21

18 years ago
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?

Comment 22

18 years ago
too sleepy to grok this tonight.  I'll look into it tomorrow am.

Comment 23

18 years ago
r=buster
I haven't actually tested it, but reading the code it looks like the right thing 
to do.  Good work.
(Assignee)

Comment 24

18 years ago
checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 25

18 years ago
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

Comment 27

10 years ago
Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/5a6def05ccbc
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.