Closed Bug 3119 Opened 26 years ago Closed 25 years ago

crash - signing in crashes apprunner

Categories

(Core :: DOM: Core & HTML, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: kerz, Assigned: karnaze)

References

()

Details

This site crashes the viewer when attempting to input user info into the form at the bottom of the page. You must not have logged into the site prior to attemting to recreate this.
per leger, assigning QA contacts to all open bugs without QA contacts according to list at http://bugzilla.mozilla.org/describecomponents.cgi?product=Browser
QA Contact: 4110 → 4137
Reassigning qa contact to cpratt@netscape.com
Chris, this look like another case of the table intermingled with form crash where a form element doesn't have an associated form frame. I'm working on generating a minimal test case (Getting close). http://blueviper/forms/submitcrash3.html For a site named insidedhtml.com, this is some of the most convoluted, nasty html I've seen - lots of unclosed and improperly nested tags...
Hmm, maybe not. It's crashing on this: <HTML> <LINK REL='stylesheet' TYPE='text/css' HREF='foo.css'> <FORM> <INPUT TYPE=submit> </FORM> </HTML> If I remove the LINK it doesn't crash. :S #0 0x40cdeae2 in main_arena () #1 0x403b53d2 in nsButtonControlFrame::MouseClicked (this=0x81e1e38, aPresContext=0x824f3b0) at nsButtonControlFrame.cpp:319 #2 0x403b5b06 in nsButtonControlFrame::HandleEvent (this=0x81e1e38, aPresContext=@0x824f3b0, aEvent=0xbffff3b4, aEventStatus=@0xbffff334) at nsButtonControlFrame.cpp:517 #3 0x402feace in PresShell::HandleEvent (this=0x8204c88, aView=0x818f0d0, aEvent=0xbffff3b4, aEventStatus=@0xbffff334) at nsPresShell.cpp:2102 #4 0x405987cc in nsView::HandleEvent (this=0x818f0d0, event=0xbffff3b4, aEventFlags=28, aStatus=@0xbffff334) at nsView.cpp:824 #5 0x405a1a88 in nsViewManager::DispatchEvent (this=0x81a4598, aEvent=0xbffff3b4, aStatus=@0xbffff334) at nsViewManager.cpp:1717 #6 0x405968c4 in HandleEvent (aEvent=0xbffff3b4) at nsView.cpp:63 #7 0x4005f452 in nsWidget::DispatchEvent (this=0x818f7b0, event=0xbffff3b4, aStatus=@0xbffff370) at nsWidget.cpp:909 #8 0x4005f35c in nsWidget::DispatchWindowEvent (this=0x818f7b0, event=0xbffff3b4) at nsWidget.cpp:871 #9 0x4005f510 in nsWidget::DispatchMouseEvent (this=0x818f7b0, aEvent=@0xbffff3b4) at nsWidget.cpp:936 #10 0x4005c46e in handle_button_release_event (w=0x815a708, event=0x817dcf0, p=0x818f7b0) at nsGtkEventHandler.cpp:541 #11 0x40a4545d in gtk_marshal_BOOL__POINTER (object=0x815a708, func=0x4005c3a8 <handle_button_release_event(_GtkWidget *, _GdkEventButton *, void *)>, func_data=0x818f7b0, args=0xbffff4a4) at gtkmarshal.c:32 #12 0x40a08dd7 in gtk_handlers_run (handlers=0x8155718, signal=0xbffff460, object=0x815a708, params=0xbffff4a4, after=0) at gtksignal.c:1909 #13 0x40a082bf in gtk_signal_real_emit (object=0x815a708, signal_id=21, params=0xbffff4a4) at gtksignal.c:1469 #14 0x40a06500 in gtk_signal_emit (object=0x815a708, signal_id=21) at gtksignal.c:552 #15 0x40a3cb18 in gtk_widget_event (widget=0x815a708, event=0x817dcf0) at gtkwidget.c:2788 #16 0x409da312 in gtk_propagate_event (widget=0x815a708, event=0x817dcf0) at gtkmain.c:1327 #17 0x409d961c in gtk_main_do_event (event=0x817dcf0) at gtkmain.c:784 #18 0x40a86b56 in gdk_event_dispatch (source_data=0x0, current_time=0xbffff828, user_data=0x0) at gdkevents.c:2086 #19 0x40ab5576 in g_main_dispatch (current_time=0xbffff828) at gmain.c:644 #20 0x40ab5ab1 in g_main_iterate (block=1, dispatch=1) at gmain.c:851 #21 0x40ab5c29 in g_main_run (loop=0x817d9e8) at gmain.c:909 #22 0x409d9065 in gtk_main () at gtkmain.c:507 #23 0x400547cc in nsAppShell::Run (this=0x8078648) at nsAppShell.cpp:208 #24 0x8052f12 in nsNativeViewerApp::Run (this=0x8070138) at nsGTKMain.cpp:42 #25 0x8053120 in main (argc=1, argv=0xbffff904) at nsGTKMain.cpp:97
Chris, would there be anything wrong with me replacing nsFormFrameTable's Put method with one that checks to make sure this entry is not a duplicate content mapping (and overwrites the previous content mapping if it is)? That is the source of the bug. Here's what's happening: Form frame (F) is created for form content (C). Form stores pointer from content (C) to frame (F) in the frame table. Elements created, look up frame pointer in the table and get (F). (cached) A style sheet is encountered. The document is reframed (even when the style sheet is not loaded :p) Form frame (G) is created for form content (C). Form appends pointer from content (C) to frame (G) to the frame table. Elements created, look up frame pointer in the table and get (F). (This is the first pointer encountered even though it is the wrong one) The original frame (F) is destroyed and removes its mapping from frame table. Now, when the form is submitted (or probably any number of other things that could be done through javascript), the elements defererence a cached pointer to frame (F). CRASH. Also, I noticed that nsFormFrameTable's Remove method creates a fixed size "HitList" array but never checks to see if that array size is exceeded when writing to the HitList. Why is the HitList needed? Would it be safe to remove it entirely? I've tested both of the above changes in my tree (including the appropriate refcounting magic) and this crash goes away. Also, I noticed that you and Kipp have been having a source code comments discussion about the accuracy improvements and possible unproven performance benefits that could be had by switching to FindFrameWithContent (or GetPrimaryFrameFor). It doesn't look like FindFrameWithContent has been moved over to a hash table yet, so are you of the opinion that I should leave in the nsFormFrameTable optimization/hack? If you want, I'd be willing to run some quick performance tests to see if it is worth changing over... I'll attach the put/get diff in a sec. (P.S. I'm not time-travelling, just felt like foregoing sleep tonight. :))
One possible solution: Index: nsFormFrame.cpp =================================================================== RCS file: /cvsroot/mozilla/layout/html/forms/src/nsFormFrame.cpp,v retrieving revision 3.28 diff -c -r3.28 nsFormFrame.cpp *** nsFormFrame.cpp 1999/04/03 22:55:07 3.28 --- nsFormFrame.cpp 1999/04/13 11:59:48 *************** *** 171,198 **** nsFormFrame& aFormFrame) { NS_ADDREF(&aFormElem); mEntries.AppendElement(new nsFormFrameTableEntry(aPresContext, aFormElem, a$ } void nsFormFrameTable::Remove(nsFormFrame& aFormFrame) { - PRInt32 hits[10]; - PRInt32 hitIndex = 0; PRInt32 count = mEntries.Count(); ! PRInt32 i; ! for (i = 0; i < count; i++) { nsFormFrameTableEntry* entry = (nsFormFrameTableEntry *)mEntries.ElementA$ if (entry->mFormFrame == &aFormFrame) { - hits[hitIndex] = i; - hitIndex++; NS_IF_RELEASE(entry->mFormElement); } } - for (i = hitIndex-1; i >= 0; i--) { - delete mEntries.ElementAt(i); - mEntries.RemoveElementAt(i); - } } --- 171,205 ---- nsFormFrame& aFormFrame) { NS_ADDREF(&aFormElem); + + // Find the FormElement if it is already in the list. + PRInt32 count = mEntries.Count(); + for (PRInt32 i = 0; i < count; i++) { + nsFormFrameTableEntry* entry = (nsFormFrameTableEntry *)mEntries.ElementA$ + if (entry->mFormElement == &aFormElem) { + NS_IF_RELEASE(entry->mFormElement); // Free up the previous reference t$ + entry->mFormElement = &aFormElem; // Create a new one + entry->mFormFrame = &aFormFrame; // Update the frame pointer + return; + } + } + mEntries.AppendElement(new nsFormFrameTableEntry(aPresContext, aFormElem, a$ } void nsFormFrameTable::Remove(nsFormFrame& aFormFrame) { PRInt32 count = mEntries.Count(); ! for (PRInt32 i = 0; i < count; i++) { nsFormFrameTableEntry* entry = (nsFormFrameTableEntry *)mEntries.ElementA$ if (entry->mFormFrame == &aFormFrame) { NS_IF_RELEASE(entry->mFormElement); + delete mEntries.ElementAt(i); + mEntries.RemoveElementAt(i); + i--; } } } *************** *** 682,687 **** --- 689,695 ---- // Hack to get the document from the parent HTML Frame. // We need this to find the base URL for submitting forms that are created in$ + // When GetDocumentURL() and GetBaseURL() are fixed this can go away. nsIDocument* nsFormFrame::GetParentHTMLFrameDocument(nsIDocument* doc) { nsIDocument* parentDocument = nsnull;
Opps, looks like the lines got chopped - but you get the idea. :)
Target Milestone: M6
Summary: Signing in crashes the viewer. → crash - signing in crashes apprunner
using build id 1999042608 on NT, this is still a crasher. talkback id: DZL46CMG
Assignee: pollmann → karnaze
Status: ASSIGNED → NEW
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Chris, you removed all of the nsFormFrameTable code today and fixed this bug! :) Reassigning to you then marking it fixed!
Status: RESOLVED → VERIFIED
Doesn't crash...
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.