crash in nsXULDocument.cpp

VERIFIED FIXED in M12

Status

()

Core
XUL
P1
normal
VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: Chris Waterson)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

In nsXULDocument.cpp, about line 3317, we have code like this:

	// Resize-reflow this time
	nsCOMPtr<nsIPresContext> cx;
	shell->GetPresContext(getter_AddRefs(cx));

	if (cx) {
		// do something
	}

	cx->GetVisibleArea(r);

If cx is null, we'll crash.

I hit this crash once last week, not exactly sure how I got there.

Updated

18 years ago
Assignee: brendan → waterson
sspitzer, did cvsblame finger me?  Prolly cuz of my whitespace-tidying anal
retention....

/be
(Assignee)

Updated

18 years ago
Priority: P3 → P1
Target Milestone: M12
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

18 years ago
-sure- brendan. :-)

Looks like a straight-forward flow-of-control problem. I'll fix it.
(Assignee)

Comment 3

18 years ago
danm: I think you made these additions to nsXULDocument::StartLayout() ages
ago. What I wonder is, do we really want the code to be as "robust" as you've
made it? i.e., seems like if we can't get a prescontext, then we should pretty
much freak out and get the hell outta dodge.
(Assignee)

Comment 4

18 years ago
Created attachment 3256 [details] [diff] [review]
proposed change
(Assignee)

Comment 5

18 years ago
danm: could you review the attached patch and let me know if it looks
reasonable?

Basically requires:

the presshell to have a prescontext,
the prescontext to have a container,
the container to be a webshell, and
the webshell to have a webshell container.

If this is not the case, it panics. Any reason this wouldn't be the case?

N.B., it does -not- require the webshell container to be an nsIBrowserWindow
(apparently this happens if we're not a top-level window? i.e., an embedded
IFRAME?)
(Assignee)

Comment 6

18 years ago
Oh duh. I am an idiot. Ignore what I've been saying about "pushing" the
controllers into the HTML element.

buster: what you need to do is set up the command dispatcher in
nsHTMLInputElement's GetControllers() method. You'll need to:

1. QI() the element's document for nsIDOMXULDocument,
2. call GetCommandDispatcher() on that
3. call SetCommandDispatcher() on your new controllers object.

See http://lxr.mozilla.org/source/mozilla/rdf/content/src/nsXULElement.cpp#3321
for code that does this. Copy n' paste, baby.

(Yeah, makes layout depend on XUL, so you might want to #ifdef INCLUDE_XUL
this. Or, like hyatt said, make the super-secret interface, but that seems
like overkill)
(Assignee)

Comment 8

18 years ago
oh damn. the last two comments were meant for a different bug.
(Assignee)

Updated

18 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

18 years ago
fix checked in.
Status: RESOLVED → VERIFIED
verified.  I looked at his checkin and we are safe now.

Comment 11

18 years ago
BULK MOVE: Changing component from XUL to XP Toolkit/Widgets: XUL.  XUL 
component will be deleted.
Component: XUL → XP Toolkit/Widgets: XUL

Updated

10 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: paulmac → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.