Closed Bug 3387 Opened 26 years ago Closed 25 years ago

Document.writes of framesets create documents with null URLs

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
All
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: morse, Assigned: vidur)

References

(
URL
)

Details

The following content displays a button. Press it and get a browser crash. Stack trace is shown below. Needless to say, this is yet another bug that is blocking development of client-side wallet. <HTML> <HEAD> <SCRIPT> function loadButtons(){ top.frames[0].document.open(); top.frames[0].document.write( "<FORM name=f>" + "<INPUT " + "type=BUTTON " + "value=\"Press me\" " + "onclick=top.frames[0].document.f.submit()>" + "</FORM>" ); top.frames[0].document.close(); } </SCRIPT> </HEAD> <FRAMESET ROWS = 25,25 onLoad=loadButtons()> <FRAME SRC=about:blank> <FRAME SRC=about:blank> </FRAMESET> <NOFRAMES> <BODY> <P> </BODY> </NOFRAMES> </HTML> NS_MakeAbsoluteURL(nsIURL * 0x00000000, const nsString & {...}, const nsString & {...}, nsString & {...}) line 1105 + 7 bytes nsFormFrame::OnSubmit(nsFormFrame * const 0x013c4da4, nsIPresContext * 0x0130bf30, nsIFrame * 0x00000000) line 535 + 28 bytes nsHTMLFormElement::Submit(nsHTMLFormElement * const 0x013bf4d0) line 291 + 18 bytes HTMLFormElementSubmit(JSContext * 0x0130d120, JSObject * 0x01875aa0, unsigned int 0, long * 0x01883f54, long * 0x0012f05c) line 363 + 15 bytes js_Invoke(JSContext * 0x0130d120, unsigned int 0, int 0) line 650 + 26 bytes js_Interpret(JSContext * 0x0130d120, long * 0x0012f834) line 2183 + 15 bytes js_Invoke(JSContext * 0x0130d120, unsigned int 1, int 0) line 666 + 13 bytes js_CallFunctionValue(JSContext * 0x0130d120, JSObject * 0x01875978, long 25647496, unsigned int 1, long * 0x0012f950, long * 0x0012f958) line 735 + 15 bytes JS_CallFunctionValue(JSContext * 0x0130d120, JSObject * 0x01875978, long 25647496, unsigned int 1, long * 0x0012f950, long * 0x0012f958) line 2370 + 29 bytes nsJSEventListener::ProcessEvent(nsIDOMEvent * 0x013c6e00) line 97 + 34 bytes nsEventListenerManager::HandleEvent(nsIPresContext & {...}, nsEvent * 0x0012fa6c, nsIDOMEvent * * 0x0012fa20, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 320 + 17 bytes nsGenericElement::HandleDOMEvent(nsIPresContext & {...}, nsEvent * 0x0012fa6c, nsIDOMEvent * * 0x0012fa20, unsigned int 1, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 748 nsHTMLInputElement::HandleDOMEvent(nsHTMLInputElement * const 0x013c0e5c, nsIPresContext & {...}, nsEvent * 0x0012fa6c, nsIDOMEvent * * 0x00000000, unsigned int 1, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 436 nsEventStateManager::CheckForAndDispatchClick(nsEventStateManager * const 0x013b5990, nsIPresContext & {...}, nsMouseEvent * 0x0012fdd0, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 379 + 31 bytes nsEventStateManager::PostHandleEvent(nsEventStateManager * const 0x013b5990, nsIPresContext & {...}, nsGUIEvent * 0x0012fdd0, nsIFrame * 0x013c4280, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 129 + 24 bytes PresShell::HandleEvent(PresShell * const 0x01373174, nsIView * 0x013c5af0, nsGUIEvent * 0x0012fdd0, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 1452 + 34 bytes nsView::HandleEvent(nsView * const 0x013c5af0, nsGUIEvent * 0x0012fdd0, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 825 nsView::HandleEvent(nsView * const 0x0132bca0, nsGUIEvent * 0x0012fdd0, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 808 nsView::HandleEvent(nsView * const 0x01374ca0, nsGUIEvent * 0x0012fdd0, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 808 nsView::HandleEvent(nsView * const 0x01374bd0, nsGUIEvent * 0x0012fdd0, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 808 nsScrollingView::HandleEvent(nsScrollingView * const 0x01374bd0, nsGUIEvent * 0x0012fdd0, unsigned int 8, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 810 nsView::HandleEvent(nsView * const 0x01372d70, nsGUIEvent * 0x0012fdd0, unsigned int 28, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 808 nsViewManager::DispatchEvent(nsViewManager * const 0x01372c60, nsGUIEvent * 0x0012fdd0, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 978 HandleEvent(nsGUIEvent * 0x0012fdd0) line 64 nsWindow::DispatchEvent(nsWindow * const 0x013c5bc0, nsGUIEvent * 0x0012fdd0, nsEventStatus & nsEventStatus_eIgnore) line 358 + 10 bytes nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012fdd0) line 374 nsWindow::DispatchMouseEvent(unsigned int 301, nsPoint * 0x00000000) line 1855 + 15 bytes nsWindow::ProcessMessage(unsigned int 514, unsigned int 0, long 852017, long * 0x0012fef4) line 1394 + 24 bytes nsWindow::WindowProc(void * 0x0300021e, unsigned int 514, unsigned int 0, long 852017) line 417 + 27 bytes
Status: NEW → ASSIGNED
on it.
Of course, it is crashing because the form has no action specified: <FORM name=f action='foo.cgi'> would be a workaround until I get a fix in... In Nav 4.5, the default action is to use the base URL. Is that doable for now?
Sorry, that doesn't prevent the crash. I'm still working on it, at any rate. :)
URL: http://blueviper/forms/submitcrash2.html
Component: Layout → Form Submission
OS: Windows NT → All
Priority: P2 → P1
Product: Mozilla → NGLayout
Summary: Pressing a submit button crashes browswer → Form submit crashes browser: nsDocument::GetBaseURL & nsDocument::GetDocumentURL
Version: 1998-03-31 → other
I'm stumped on this one. Seems like the bug is not limited to forms. Peter, since you wrote nsDocument::GetBaseURL maybe you can explain to me how it is being (mis)used by nsFormFrame::OnSubmit, or point me in the right direction? In this example, a frame is created by javascript. The frame contains a form. When the form is submitted, nsFormFrame::OnSubmit queries nsDocument::GetBaseURL() to construct the URL to submit to. Since the frame was created by JS, it doesn't have a URL and GetBaseURL() returns nsnull. The forms code was not handling this properly. I wrote a work-around for nsFormFrame. To get around the crash, I had the form code check for a null BaseURL. If it sees a null, it gets the parent frame's document (through many layers of indirection) and uses that BaseURL. (I made a utility method called nsFormFrame::GetParentHTMLFrameDocument that is called in two places) However, even the workaround doesn't solve the problem! The crash no longer occurs in forms, but when the URL is loading, nsWebShell::DoLoadURL calls nsDocument::GetDocumentURL which returns null, causing a crash there. It seems that in many places throughout the code, we are assuming that GetBaseURL and GetDocumentURL return valid URLs. Is there some way to ensure a valid URL, even when the document in question is created by javascript? To see Steve's test case go to: http://blueviper/forms/submitcrash2.html To see my form frame diffs (and admitted overuse of QI), see: http://blueviper/forms/getbaseurl.txt
After making a one-line change to nsWebShell.cpp (check for null), I found that the crash went away. I'll be checking in the changes later today when the tree opens. Apply the diff at http://blueviper/forms/getbaseurl.txt and the following, and you should be golden: Diff for nsWebShell.cpp: pollmann blueviper(44):/builds/ng/mozilla/webshell/src> cvs diff -w cvs server: Diffing . Index: nsWebShell.cpp =================================================================== RCS file: /cvsroot/mozilla/webshell/src/nsWebShell.cpp,v retrieving revision 1.103 diff -w -r1.103 nsWebShell.cpp 1276a1277 > if (docURL) { // Don't assume docURL is a valid URL: could be a js created page. 1288a1290 > } cvs server: Diffing windows pollmann blueviper(45):/builds/ng/mozilla/webshell/src>
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
Steve, I see you checked in the GetParentHTMLFrameDocument routine. Have you also checked in the other diffs that I sent you? (including the patch in webshell.cpp?) If so, can I close this bug out?
I checked in the changed in webshell.cpp, formframe.h, formframe.cpp. Please check that I didn't miss anything before you close this out. Sorry that I jumped the gun on you but I had to add these in order to get wallet to work.
No problem. Thanks, actually - you saved me having to watch the tree! :)
Assignee: pollmann → nisheeth
Status: ASSIGNED → NEW
Nisheeth, as you are the new nsWebShell guru... There is an assumption that nsDocument::GetDocumentURL will return a non-null in nsWebShell.cpp at line 1308. This causes viewer to segfault on the above URL. I made a change that gets past this crash, but I'm not sure if it is the "right thing" to do for nsWebShell. The patch is in my 3/3 comment, but was never checked in. Can you take over this change? BTW, this change causes links to name anchors within a javascript document to reload the entire document. This may or probably won't be desirable which is why I'm passing it on to you. CC'ing Harish as this parser fix he's working on is required to see this bug.
Status: NEW → ASSIGNED
Eric, you are right in that the real problem is that documents created by document.writes have a null URL. Vidur coded up a temporary fix that assigns "about:blank" as the URL for JS written documents. I've checked in that fix into the M3 branch and the tip. I will look at the 4.x codebase and see how URLs for document.written documents were assigned and replicate that behavior in Gecko. It might make sense to get rid of the code that you added in nsFormFrame that handles the case of documents with null urls. What do you think?
Does this fix break the above test case? (It would seem to) Overall, I agree that this type of fix is certainly the way to go. However, using "about:blank" will cause problems in forms. When a form is submitted, GetBaseURL is called to help generate a fully-qualified URL from the form ACTION property. For example, I may have specified "getmap.cgi" as the action of a js generated form in an HTML document. When this form is submitted, we don't have any idea what domain or path this CGI is at. That's why I wrote the method GetParentHTMLFrameDocument(). This way, if the URL comes back NULL, we just go one up the HTML frame chain to find out what the URL of the parent is, and generate an absolute URL from that. GetParentHTMLFrameDocument is not a solution and should go away. The problem is more general. For example, the generated document could be in a separate window - in which case even GetParentHTMLFrameDocument will fail. What we need is to get access to the URL of the document that generated this one (if this one is has not valid URL) whether that document be a containing frame, in a separate window, or whatever.
Yes, the about:blank fix was temporary. I want to find out what Nav 4.x does and emulate that. I'm guessing it sets the URL to that of the writing document.
Target Milestone: M4
Setting milestone to M4...
*** Bug 3584 has been marked as a duplicate of this bug. ***
Target Milestone: M4 → M5
Moving bug to M5...
Assignee: nisheeth → vidur
Status: ASSIGNED → NEW
Component: Form Submission → DOM Level 0
Summary: Form submit crashes browser: nsDocument::GetBaseURL & nsDocument::GetDocumentURL → Document.writes of framesets create documents with null URLs
Updating summary. Setting component to DOM Level 0. Re-assigning to Vidur.
Target Milestone: M5 → M6
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Fix checked in on 5/18/1999. Document.opened pages now get the same URL as their source as per Nav 4.x.
Status: RESOLVED → VERIFIED
I don't see a browser crash at the above URL, or at the URL mentioned in bug 3584. This is using linux and the 1999052408 build.
You need to log in before you can comment on or make changes to this bug.