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)

Product:
Core
Component:
XUL
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME
Milestone:
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
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.
Severity: normal → major
Status: NEW → ASSIGNED
Keywords: crash, rtm
Priority: P3 → P1
Summary: 2nd Browse in composer window givse blank window → 2nd Browse in composer window crashes
Target Milestone: --- → M18
We're crashing in nsDSURIContentListener::OnStartURIOpen because 
mParentContentListener is bad.

This is the JS that opens the browser window to preview:

	    window.openDialog(getBrowserURL(), "EditorPreview", "chrome,all,dialog=
no", window._content.location);

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?

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.

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]
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.

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.

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);

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?

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.
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++]
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++]
Target Milestone: M18 → mozilla1.0
*** Bug 65554 has been marked as a duplicate of this bug. ***
Any chance of getting this for Mozilla 0.9 ?

nominating for nsbeta1 (hoping for a mozilla0.9).  I get this crash all the time.
Keywords: nsbeta1
*** Bug 99900 has been marked as a duplicate of this bug. ***
Blocks: 104166
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
Target Milestone: mozilla1.0.1 → ---
Target Milestone: --- → mozilla1.0.1
Target Milestone: mozilla1.0.1 → mozilla1.1beta
Target Milestone: mozilla1.1beta → Future
Update target milestone to 'Future' since this missed the 'mozilla1.1beta' train.
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
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. 
Not seeing this problem.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040316
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.