Closed Bug 11490 Opened 21 years ago Closed 21 years ago

Crash in cookie viewer

Categories

(Core :: Layout, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: morse, Assigned: rods)

Details

Attachments

(2 files)

Go to any page that sets cookies (e.g., home.netscape.com) and allow the cookie
to be set.  Then open up the cookie viewer (menu item
edit/wallet/display-cookies).  Select one of the cookies and press delete.  You
get the following crash.

Note: At this time the cookie viewer is executing javascript code.  That code
can be found in extensions/wallet/cookie-viewer/CookieViewer.html.


nsCSSFrameConstructor::ContentRemoved(nsCSSFrameConstructor * const 0x0abb1a90,
nsIPresContext * 0x0ab8a3f0, nsIContent * 0x0ac91770, nsIContent * 0x0ac9429c,
int 1) line 4831 + 7 bytes
StyleSetImpl::ContentRemoved(StyleSetImpl * const 0x0abb1b30, nsIPresContext *
0x0ab8a3f0, nsIContent * 0x0ac91770, nsIContent * 0x0ac9429c, int 1) line 818
PresShell::ContentRemoved(PresShell * const 0x0abb19a8, nsIDocument *
0x0ab89860, nsIContent * 0x0ac91770, nsIContent * 0x0ac9429c, int 1) line 1601 +
50 bytes
nsDocument::ContentRemoved(nsDocument * const 0x0ab89860, nsIContent *
0x0ac91770, nsIContent * 0x0ac9429c, int 1) line 1632
nsHTMLDocument::ContentRemoved(nsHTMLDocument * const 0x0ab89860, nsIContent *
0x0ac91770, nsIContent * 0x0ac9429c, int 1) line 894
nsGenericHTMLContainerElement::RemoveChildAt(int 1, int 1) line 2794
nsGenericHTMLContainerElement::RemoveChild(nsIDOMNode * 0x0ac94290, nsIDOMNode *
* 0x0012c284) line 2604 + 14 bytes
nsHTMLSelectElement::RemoveChild(nsHTMLSelectElement * const 0x0ac91760,
nsIDOMNode * 0x0ac94290, nsIDOMNode * * 0x0012c284) line 120 + 22 bytes
nsHTMLSelectElement::Remove(nsHTMLSelectElement * const 0x0ac91760, int 1) line
347
HTMLSelectElementRemove(JSContext * 0x0ac4d860, JSObject * 0x0a28b960, unsigned
int 1, long * 0x0a258de8, long * 0x0012c370) line 592 + 19 bytes
js_Invoke(JSContext * 0x0ac4d860, unsigned int 1, unsigned int 0) line 654 + 26
bytes
js_Interpret(JSContext * 0x0ac4d860, long * 0x0012cb9c) line 2228 + 15 bytes
js_Invoke(JSContext * 0x0ac4d860, unsigned int 0, unsigned int 0) line 670 + 13
bytes
js_Interpret(JSContext * 0x0ac4d860, long * 0x0012d384) line 2228 + 15 bytes
js_Invoke(JSContext * 0x0ac4d860, unsigned int 0, unsigned int 0) line 670 + 13
bytes
js_Interpret(JSContext * 0x0ac4d860, long * 0x0012db6c) line 2228 + 15 bytes
js_Invoke(JSContext * 0x0ac4d860, unsigned int 1, unsigned int 2) line 670 + 13
bytes
js_InternalCall(JSContext * 0x0ac4d860, JSObject * 0x0a28ba30, long 170441272,
unsigned int 1, long * 0x0012dcac, long * 0x0012dcb4) line 747 + 15 bytes
JS_CallFunctionValue(JSContext * 0x0ac4d860, JSObject * 0x0a28ba30, long
170441272, unsigned int 1, long * 0x0012dcac, long * 0x0012dcb4) line 2643 + 29
bytes
nsJSEventListener::HandleEvent(nsIDOMEvent * 0x0acb3480) line 97 + 34 bytes
nsEventListenerManager::HandleEvent(nsIPresContext & {...}, nsEvent *
0x0012df20, nsIDOMEvent * * 0x0012ddec, unsigned int 3, nsEventStatus &
nsEventStatus_eIgnore) line 601 + 21 bytes
nsGenericElement::HandleDOMEvent(nsIPresContext & {...}, nsEvent * 0x0012df20,
nsIDOMEvent * * 0x0012ddec, unsigned int 1, nsEventStatus &
nsEventStatus_eIgnore) line 784
nsHTMLButtonElement::HandleDOMEvent(nsHTMLButtonElement * const 0x0ac9d56c,
nsIPresContext & {...}, nsEvent * 0x0012df20, nsIDOMEvent * * 0x00000000,
unsigned int 1, nsEventStatus & nsEventStatus_eIgnore) line 398 + 31 bytes
nsEventStateManager::CheckForAndDispatchClick(nsEventStateManager * const
0x0abb4640, nsIPresContext & {...}, nsMouseEvent * 0x0012e198, nsEventStatus &
nsEventStatus_eIgnore) line 735 + 31 bytes
nsEventStateManager::PostHandleEvent(nsEventStateManager * const 0x0abb4640,
nsIPresContext & {...}, nsGUIEvent * 0x0012e198, nsIFrame * 0x0aca0be0,
nsEventStatus & nsEventStatus_eIgnore, nsIView * 0x0ac74b50) line 261 + 24 bytes
PresShell::HandleEvent(PresShell * const 0x0ac74754, nsIView * 0x0ac74b50,
nsGUIEvent * 0x0012e198, nsEventStatus & nsEventStatus_eIgnore) line 1882 + 43
bytes
nsView::HandleEvent(nsView * const 0x0ac74b50, nsGUIEvent * 0x0012e198, unsigned
int 28, nsEventStatus & nsEventStatus_eIgnore, int & 0) line 834
nsViewManager::DispatchEvent(nsViewManager * const 0x0ac74d70, nsGUIEvent *
0x0012e198, nsEventStatus & nsEventStatus_eIgnore) line 1611
HandleEvent(nsGUIEvent * 0x0012e198) line 67
nsWindow::DispatchEvent(nsWindow * const 0x0ac74a14, nsGUIEvent * 0x0012e198,
nsEventStatus & nsEventStatus_eIgnore) line 498 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012e198) line 523
nsWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 3268 +
21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line
3463
nsWindow::ProcessMessage(unsigned int 514, unsigned int 0, long 3211299, long *
0x0012e3cc) line 2536 + 24 bytes
nsWindow::WindowProc(HWND__ * 0x032507b6, unsigned int 514, unsigned int 0, long
3211299) line 571 + 27 bytes
USER32! 77e71268()
00310023()
Assignee: troy → pollmann
Please provide a reduced test case.
Attached file reduced test case
Reduced test case is attached.  Select the item in the list and press delete.
Crash occurs on the javascript line selname.remove(i-1).
Assignee: pollmann → rods
Rod, accoridng to CVS this is code that you added to ContentRemoved() for SELECT
frames it looks like.

Why is this code even here in the first place? We can't just go scattering code
all over the place. The frame construction code is such a mess...
Attached file reduced test case
Troy, I added this code in the nsCSSFrameConstructor in ContentRemoved,
ContentAppended, and ContentInserted, because Peter suggested this was the best
approach to this problem.

The reason it crashed was because aContainer was null in the call
to ContentRemoved, I didn't think that could ever be null? Does that make sense
that that is null?
I don't think adding SELECT specific code to ContentRemoved, ContentInserted,
ContntAppended is a very good idea. Imagine what a mess we'll have if we start
doing that for each type of frame

Why can't the SELECT frame code do somethiung sensible in the case where it has
no child frames?

As far as aContainer being NULL, I don't think we epected it that originally,
but it seems as if it happens that way now. Evidently it's because someone is
removing the root content node.

I expect that there are other places in that function that may not be checking
for a NULL pointer either. If so assign the bug back to me after fixing the
SELECT related code
Well, it is doing something sensible, it is using generated content to create a
frame when there are no children, the frame is neccessary so layout can
calculate the correct size for the select when there are no children. The extra
code that was added was to remove and add the dummy frame when the list is
dynamically updated. Since, Peter instructed me to put the code in the
nsCSSFrameConstructor I will let him explain why this is the best place for it.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Marking this as fix, but the discussion is still useful.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
No, it is not fixed!  And it's important for M9.  The bug that you are thinking
about that was fixed was causing a crash when the cookie viewer was brought up.
The fix was in xpcom/ds/nsSupportsArray.cpp which I made on Sunday.  There was
no bug report filed on it.

This bug causes nothing to happen when you attempt to delete a cookie.

Reopening.
Correction.  I said it causes nothing to happen when you press delete.  No, it
crashes at that point.
I found the proble and it is an easy fix. I misunderstodd what the actual bug
was about and that it had to do with native widgets and not GFX. It works for
GFX and I will be checking in small fix to make it work again under native.
That's not how generated content works. You don't "decide" to add some because
you don't have any child frames left.

The problem with hacks like this is it increases the amount of code and creates
a maintenance problem
Fixed it no longer crashes for native or gfx comboxboxes
Status: REOPENED → RESOLVED
Closed: 21 years ago21 years ago
Resolution: --- → FIXED
Confirmed.  The cookie viewer no longer crashes.
Status: RESOLVED → VERIFIED
Based on the reporter's comments, marking as verified fixed.
You need to log in before you can comment on or make changes to this bug.