{ib} page crashes browser

RESOLVED WORKSFORME

Status

()

Core
Layout
P1
critical
RESOLVED WORKSFORME
16 years ago
16 years ago

People

(Reporter: Tormod V, Assigned: Chris Waterson)

Tracking

({crash, testcase})

Trunk
Future
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:0.9.9+) Gecko/20020326
BuildID:    2002032608

The above URL crashes the browser after displaying it. If javascript is
disabled, it does not crash.

Reproducible: Always
Steps to Reproduce:
1. Go to the URL, or go to http://www.davos.ch (which does not crash) then click
the "News" link on the right.


Actual Results:  Page loads, displays and mozilla dies.

Expected Results:  Page loads and displays.

Talkback TB4571918M. I have tried to isolate the bad js code by editing a local
copy of the html. If I disable the "shForm" in the body onload, and the loading
of "HM_Loader.js", it does not crash. But now I am too tired...

Comment 1

16 years ago
Confirming crash with Mozilla trunk debug build 2002-03-24 WinNT.
OS: Mac9.x ---> All.

Will attach stack trace below -
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac System 9.x → All
Hardware: Macintosh → All

Comment 2

16 years ago
Created attachment 76601 [details]
WinNT stack trace

Comment 3

16 years ago
Stack trace looks like it has to do with DOM CSS; therefore 
reassigning to DOM Style component for further triage -
Assignee: rogerl → jst
Component: JavaScript Engine → DOM Style
QA Contact: pschwartau → ian
The page is changing visibility on a "ilayer" tag, and that ends up doing a
stylechange reflow which ends up finding a reference to a deleted frame in the
frame tree and we crash trying to call QI on the deleted frame.

Over to layout.
Assignee: jst → attinasi
Component: DOM Style → Layout
QA Contact: ian → petersen

Updated

16 years ago
Keywords: crash
Created attachment 77462 [details]
Testcase: click to crash

This is based on a crasher page I was given. Talkback ID TB4771799W. If the
stack trace matches then this is the bug -- the descriptions sounds similar.

Comment 6

16 years ago
Thanks Chris!
Assignee: attinasi → waterson
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 7

16 years ago
I wasn't able to crash with today's build using the test case; however, the
original URL still crashes after the page has loaded.
(Assignee)

Comment 8

16 years ago
Created attachment 80081 [details]
revised testcase, click on content area to crash.
Attachment #77462 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Summary: page crashes browser → {ib} page crashes browser
(Assignee)

Comment 9

16 years ago
Mmm, more block-in-inline fun. With an <hr> in the third part of the spit,
changing the visibility on the split hierarchy causes a crash.
(Assignee)

Comment 10

16 years ago
Created attachment 80117 [details] [diff] [review]
proposed fix, of the wallpaper variety.

Boy, we really need to re-do {ib}.

So here's what's happening. The <span> containing the <div> is being targeted
for a style change; specifically, the visibility is being changed via the style
DOM. We walk the {ib} siblings to propagate the style changes to those frames
as well; inside one of the siblings, a frame change is triggered for one of the
children (who knows why -- maybe this is another bug). Regardless, this has the
effect of adding the child to the change list with, but not updating the
top-level maxHint. When we ProcessRestyledFrames, the frame that wants a
reframe triggers the ReframeContainingBlock logic: this nukes _all_ the other
frames in the change list. So, when we try to process them, they've been
deleted, and we crash.

The fix is to inspect the change list after adding each of the {ib} siblings.
If we ever get into a situation where someone is going to get a frame change,
we bail, and reframe the whole kit and kaboodle.
(Assignee)

Comment 11

16 years ago
Review-shopping for some {ib} spackle...
Keywords: review
So this really seems quite wall-papery, considering AttributeChanged is by no
means the only caller of ComputeStyleChangeFor.  It also seems a bit too
agressive (since we really only need to do this if it's something that causes
|WipeContainingBlock| to be called, i.e., a frame that IS_FRAME_SPECIAL,
right?).  Wouldn't a cleaner fix be to move the |ReframeContainingBlock| logic
into the style change mechanism somehow, so that we send the reframe to the top
frame to begin with?  But r=dbaron if you want.

Now that you mention it, there probably is an easier way to rewrite {ib} stuff...
(Assignee)

Comment 13

16 years ago
Ugh, I didn't look at all the other callers of ComputeStyleChangeFor. So you're
right, this is going to stop one kind of crash, but not four (or more?) others.
I'll see if I can fold the necessary {ib} logic into
nsFrameManager::CopmuteStyleChangeFor and ReResolveStyleContext.

Marc was right. This summer we ought to try to re-think this whole mess.
(Assignee)

Updated

16 years ago
Attachment #80117 - Flags: needs-work+
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.0 → mozilla1.0.1

Updated

16 years ago
Keywords: testcase
Target Milestone: mozilla1.0.1 → mozilla1.1beta
(Assignee)

Updated

16 years ago
Target Milestone: mozilla1.1beta → Future

Comment 14

16 years ago
Reporter,

Please try with a new nighly build I can reproduce the crash on site or reduced
test case provided. Tested under OSX with 2002-07-31-05 1.0.
(Reporter)

Comment 15

16 years ago
WFM :) Does not crash using the URL and the testcases, using mozilla-mac-trunk
Build ID 2002080108.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.