Closed
Bug 56157
Opened 24 years ago
Closed 20 years ago
window.openDialog loading into an existing named window crashes or fails.
Categories
(Core :: XUL, defect, P1)
Core
XUL
Tracking
()
RESOLVED
WORKSFORME
Future
People
(Reporter: sujay, Assigned: danm.moz)
References
Details
(Keywords: crash)
using 10/11 build of netscape 1) launch netscape 2) launch composer 3) insert text 4) save 5) browse 6) add some more text 7) save 8) browse again notice you get a blank window when you Browse a 2nd time
Comment 1•24 years ago
|
||
On Mac, the second time you 'Browser' *but keep the first Browser window open* then you crash. It locked up the machine for me; serious bug.
Comment 2•24 years ago
|
||
We're crashing in nsDSURIContentListener::OnStartURIOpen because mParentContentListener is bad.
Comment 3•24 years ago
|
||
This is the JS that opens the browser window to preview: window.openDialog(getBrowserURL(), "EditorPreview", "chrome,all,dialog= no", window._content.location);
Comment 4•24 years ago
|
||
The crash appears to happen because the nsBrowserInstance is not doing a SetParentContentListener(nsnull) at an appropriate time. danm, is this related to that ReinitializeContentVariables() change that you made recently?
Comment 5•24 years ago
|
||
Well, I can make it not crash: Index: mozilla/xpfe/browser/src/nsBrowserInstance.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/browser/src/nsBrowserInstance.cpp,v retrieving revision 1.166.2.1 diff -r1.166.2.1 nsBrowserInstance.cpp 1492a1493,1495 > if (mDocShell) > mDocShell->SetParentURIContentListener(nsnull); > but it still comes up blank the second time. An alternative would be to have Preview bring up a new window each time: Index: mozilla/editor/ui/composer/content/ComposerCommands.js =================================================================== RCS file: /cvsroot/mozilla/editor/ui/composer/content/ComposerCommands.js,v retrieving revision 1.36.4.1 diff -r1.36.4.1 ComposerCommands.js 461c461 < window.openDialog(getBrowserURL(), "EditorPreview", "chrome,all,dialog= no", window._content.location); --- > window.openDialog(getBrowserURL(), "_blank", "chrome,all,dialog=no", window._content.location);
Whiteboard: [rtm+]
As far as I can tell my recent change to ReinitializeContentVariables() isn't being triggered in this scenario, so it seems unrelated. This is intriguing -- on the second "Browse" invocation the window is being torn down and it looks like it's not torn down completely. Your proposed patch (the first one) seems like the important part of what would happen if you called SetWebShellWindow(0) within Close (SWSW expects a non-null parameter), so it makes sense. I am way worried about the implication that there may be lots of other cruft we don't know about that wants clearing out. I guess I'd recommend doing the first patch (much nicer UI) in the trunk and the second patch (much safer) in the rtm branch.
Comment 7•24 years ago
|
||
Looks like danm is signing up for r=danm on the second patch, but you still need an a=/sr=. Marking [rtm need info]
Whiteboard: [rtm+] → [rtm need info]
Comment 8•24 years ago
|
||
Neither of my patches (even both) really fix the bug; it can either not crash and show a blank window, or bring up a new browser window each time. But we want to reuse the existing preview window if one is open.
Comment 9•24 years ago
|
||
Well, I notice a lot of bugs in this code; possible leaks: Index: mozilla/dom/src/base/nsGlobalWindow.cpp =================================================================== RCS file: /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v retrieving revision 1.351.2.3 diff -u -2 -r1.351.2.3 nsGlobalWindow.cpp --- nsGlobalWindow.cpp 2000/10/06 00:26:18 1.351.2.3 +++ nsGlobalWindow.cpp 2000/10/12 21:43:26 @@ -3443,5 +3443,5 @@ nsCOMPtr<nsIScriptGlobalObject> globalObject(do_GetInterface(aDocShellItem)); NS_ENSURE_TRUE(globalObject, NS_ERROR_FAILURE); - res = CallQueryInterface(globalObject.get(), aDOMWindow); + res = CallQueryInterface(globalObject.get(), aDOMWindow); // leak? globalObject->SetOpenerWindow(this); // damnit return res; and failures to addref: Index: mozilla/xpfe/appshell/src/nsChromeTreeOwner.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsChromeTreeOwner.cpp,v retrieving revision 1.18 diff -u -2 -r1.18 nsChromeTreeOwner.cpp --- nsChromeTreeOwner.cpp 2000/09/14 22:56:54 1.18 +++ nsChromeTreeOwner.cpp 2000/10/12 21:44:14 @@ -135,5 +135,5 @@ xulWindow->GetPrimaryContentShell(getter_AddRefs(shellAsTreeItem)); if(shellAsTreeItem) - *aFoundItem = shellAsTreeItem; + NS_ADDREF(*aFoundItem = shellAsTreeItem); } else Index: mozilla/xpfe/appshell/src/nsContentTreeOwner.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsContentTreeOwner.cpp,v retrieving revision 1.30 diff -u -2 -r1.30 nsContentTreeOwner.cpp --- nsContentTreeOwner.cpp 2000/09/14 11:09:51 1.30 +++ nsContentTreeOwner.cpp 2000/10/12 21:44:14 @@ -142,5 +142,5 @@ { if(fIs_Content) - *aFoundItem = shellAsTreeItem; + NS_ADDREF(*aFoundItem = shellAsTreeItem); else if(aRequestor != shellAsTreeItem.get()) { Fixing these does not help the problem, though.
Comment 10•24 years ago
|
||
I think the safest fix for this now is to open a new browser window for each
preview. Patch is:
Index: mozilla/editor/ui/composer/content/ComposerCommands.js
===================================================================
RCS file: /cvsroot/mozilla/editor/ui/composer/content/ComposerCommands.js,v
retrieving revision 1.36.4.1
diff -r1.36.4.1 ComposerCommands.js
461c461
< window.openDialog(getBrowserURL(), "EditorPreview",
"chrome,all,dialog=
no", window._content.location);
---
> window.openDialog(getBrowserURL(), "_blank", "chrome,all,dialog=no",
window._content.location);
Comment 11•24 years ago
|
||
ugh! If only we had more time... if only... r=brade but only for the N6 branch cc kin for sr=kin We really need to investigate this for the trunk and understand why it doesn't work. Why does it appear that bookmarks, sidebar, etc. are able to load content into the browser repeatedly and Composer can't?
Comment 12•24 years ago
|
||
sr=kin@netscape.com on sfraser's conservative "_blank" workaround. Lets get the workaround checked in on the Netscape_20000922_BRANCH and then reassign this bug to folks in xpfe to fix the crasher.
Comment 13•24 years ago
|
||
Changing to rtm-double-plus. Crashing is bad... the proposed one-liner looks safe and well reviewed. Simon sent email requesting PDT consideration, and that is why I'm making the change from need-info to double-plus. Usually folks change to a plus to attract PDT.
Whiteboard: [rtm need info] → [rtm++]
Comment 14•24 years ago
|
||
Fixed checked into branch. Giving bug to danm for a real fix on the trunk.
Assignee: sfraser → danm
Status: ASSIGNED → NEW
Component: Editor → XP Toolkit/Widgets
Keywords: rtm
Summary: 2nd Browse in composer window crashes → window.openDialog loading into an existing named window crashes or fails.
Whiteboard: [rtm++]
Comment 15•24 years ago
|
||
*** Bug 65554 has been marked as a duplicate of this bug. ***
Comment 16•24 years ago
|
||
Any chance of getting this for Mozilla 0.9 ?
Comment 17•24 years ago
|
||
nominating for nsbeta1 (hoping for a mozilla0.9). I get this crash all the time.
Keywords: nsbeta1
Comment 18•23 years ago
|
||
*** Bug 99900 has been marked as a duplicate of this bug. ***
Comment 19•23 years ago
|
||
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 (you can query for this string to delete spam or retrieve the list of bugs I've moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
Updated•22 years ago
|
Target Milestone: mozilla1.1beta → Future
Comment 20•22 years ago
|
||
Update target milestone to 'Future' since this missed the 'mozilla1.1beta' train.
Comment 21•22 years ago
|
||
By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and <http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss bugs are of critical or possibly higher severity. Only changing open bugs to minimize unnecessary spam. Keywords to trigger this would be crash, topcrash, topcrash+, zt4newcrash, dataloss.
Severity: major → critical
Comment 22•21 years ago
|
||
I'm using the ver 1.3 of Mozilla. window.openDialog fails to initiate a dialog. I get the error - Error: uncaught exception: [Exception... "Security error" code: "1000" nsresult: "0x805303e8 (NS_ERROR_DOM_SECURITY_ERR)" location: "<unknown>"] - in the Javascript console. I can open the dialog with window.open - but I'm unable to pass arguments.
Comment 23•20 years ago
|
||
Not seeing this problem. Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040316
Comment 24•20 years ago
|
||
Unable to reproduce with Mozilla 1.7 beta on windows XP
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•