Fixing bug 389634. Remove dependency on XUL windows from modal content dialog opening code (window.showModalDialog()). r=bzbarksy@mit.edu, sr=jonas@sicking.cc

docshell/base/nsDocShell.cpp

@@ -8502,15 +8502,20 @@ nsDocShell::EnsureScriptEnvironment()
         do_GetService(kDOMScriptObjectFactoryCID);
     NS_ENSURE_TRUE(factory, NS_ERROR_FAILURE);
 
-    nsCOMPtr<nsIDocShellTreeItem> parent;
-    GetParent(getter_AddRefs(parent));
+    nsCOMPtr<nsIWebBrowserChrome> browserChrome(do_GetInterface(mTreeOwner));
+    NS_ENSURE_TRUE(browserChrome, NS_ERROR_NOT_AVAILABLE);
 
-    nsCOMPtr<nsPIDOMWindow> pw(do_GetInterface(parent));
+    PRUint32 chromeFlags;
+    browserChrome->GetChromeFlags(&chromeFlags);
 
-    // If the parent (chrome or not) is a modal content window, make
-    // this window a modal content window as well.
+    PRBool isModalContentWindow =
+        (chromeFlags & nsIWebBrowserChrome::CHROME_MODAL) &&
+        !(chromeFlags & nsIWebBrowserChrome::CHROME_OPENAS_CHROME);
+
+    // If our window is modal and we're not opened as chrome, make
+    // this window a modal content window.
     factory->NewScriptGlobalObject(mItemType == typeChrome,
-                                   pw && pw->IsModalContentWindow(),
+                                   isModalContentWindow,
                                    getter_AddRefs(mScriptGlobal));
     NS_ENSURE_TRUE(mScriptGlobal, NS_ERROR_FAILURE);
 

dom/src/base/nsGlobalWindow.cpp

@@ -5111,6 +5111,10 @@ nsGlobalWindow::GetFrameElement(nsIDOMElement** aFrameElement)
   return NS_OK;
 }
 
+// Helper for converting window.showModalDialog() options (list of ';'
+// separated name (:|=) value pairs) to a format that's parsable by
+// our normal window opening code.
+
 void
 ConvertDialogOptions(const nsAString& aOptions, nsAString& aResult)
 {
@@ -5239,12 +5243,15 @@ nsGlobalWindow::ShowModalDialog(const nsAString& aURI, nsIVariant *aArgs,
 
   nsCOMPtr<nsIDOMWindow> dlgWin;
   nsAutoString options(NS_LITERAL_STRING("modal=1,status=1"));
-  nsAutoString dialogOptions(aOptions);
 
-  ConvertDialogOptions(dialogOptions, options);
+  ConvertDialogOptions(aOptions, options);
 
   options.AppendLiteral(",scrollbars=1,centerscreen=1,resizable=0");
 
+  // Before bringing up the window, unsuppress painting and flush
+  // pending reflows.
+  EnsureReflowFlushAndPaint();
+
   nsresult rv = OpenInternal(aURI, EmptyString(), options,
                              PR_FALSE,          // aDialog
                              PR_TRUE,           // aCalledNoScript
@@ -5253,7 +5260,7 @@ nsGlobalWindow::ShowModalDialog(const nsAString& aURI, nsIVariant *aArgs,
                              GetPrincipal(),    // aCalleePrincipal
                              nsnull,            // aJSCallerContext
                              getter_AddRefs(dlgWin));
-  if (NS_FAILED(rv))
+  if (NS_FAILED(rv) || !dlgWin)
     return NS_OK;
 
   nsCOMPtr<nsPIDOMWindow> win(do_QueryInterface(dlgWin));

embedding/components/windowwatcher/src/nsPrompt.cpp

@@ -176,24 +176,6 @@ nsPrompt::Init()
 // nsPrompt::nsIPrompt
 //*****************************************************************************
 
-class nsAutoWindowStateHelper
-{
-public:
-  nsAutoWindowStateHelper(nsIDOMWindow *aWindow);
-  ~nsAutoWindowStateHelper();
-
-  PRBool DefaultEnabled()
-  {
-    return mDefaultEnabled;
-  }
-
-protected:
-  PRBool DispatchCustomEvent(const char *aEventName);
-
-  nsIDOMWindow *mWindow;
-  PRBool mDefaultEnabled;
-};
-
 nsAutoWindowStateHelper::nsAutoWindowStateHelper(nsIDOMWindow *aWindow)
   : mWindow(aWindow),
     mDefaultEnabled(DispatchCustomEvent("DOMWillOpenModalDialog"))
@@ -228,9 +210,6 @@ nsAutoWindowStateHelper::DispatchCustomEvent(const char *aEventName)
 #ifdef DEBUG
   {
     nsCOMPtr<nsPIDOMWindow> window(do_QueryInterface(mWindow));
-
-    NS_ASSERTION(window->GetExtantDocument() != nsnull,
-                 "nsPrompt used too early on window object!");
   }
 #endif
 

embedding/components/windowwatcher/src/nsPrompt.h

@@ -79,6 +79,30 @@ protected:
   nsCOMPtr<nsIPromptService2>   mPromptService2;
 };
 
+
+/**
+ * Helper class for dealing with notifications around opening modal
+ * windows.
+ */
+class nsAutoWindowStateHelper
+{
+public:
+  nsAutoWindowStateHelper(nsIDOMWindow *aWindow);
+  ~nsAutoWindowStateHelper();
+
+  PRBool DefaultEnabled()
+  {
+    return mDefaultEnabled;
+  }
+
+protected:
+  PRBool DispatchCustomEvent(const char *aEventName);
+
+  nsIDOMWindow *mWindow;
+  PRBool mDefaultEnabled;
+};
+
+
 /**
  * A class that wraps an nsIAuthPrompt so that it can be used as an
  * nsIAuthPrompt2.

embedding/components/windowwatcher/src/nsWindowWatcher.cpp

@@ -546,14 +546,6 @@ nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow *aParent,
 
   nsCOMPtr<nsIDOMChromeWindow> chromeParent(do_QueryInterface(aParent));
 
-  // If we're not called through our JS version of the API, and we got
-  // a modal option, treat the window we're opening as a modal content
-  // window.
-  if (!aCalledFromJS && argv &&
-      WinHasOption(features.get(), "modal", 0, nsnull)) {
-    windowIsModalContentDialog = PR_TRUE;
-  }
-
   // Make sure we call CalculateChromeFlags() *before* we push the
   // callee context onto the context stack so that
   // CalculateChromeFlags() sees the actual caller when doing it's
@@ -562,6 +554,11 @@ nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow *aParent,
                                      aDialog, uriToLoadIsChrome,
                                      !aParent || chromeParent);
 
+  if ((chromeFlags & nsIWebBrowserChrome::CHROME_MODAL) &&
+      !(chromeFlags & nsIWebBrowserChrome::CHROME_OPENAS_CHROME)) {
+    windowIsModalContentDialog = PR_TRUE;
+  }
+
   SizeSpec sizeSpec;
   CalcSizeSpec(features.get(), sizeSpec);
 
@@ -673,8 +670,9 @@ nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow *aParent,
 
         PRBool cancel = PR_FALSE;
         rv = windowCreator2->CreateChromeWindow2(parentChrome, chromeFlags,
-                               contextFlags, uriToLoad, &cancel,
-                               getter_AddRefs(newChrome));
+                                                 contextFlags, uriToLoad,
+                                                 &cancel,
+                                                 getter_AddRefs(newChrome));
         if (NS_SUCCEEDED(rv) && cancel) {
           newChrome = 0; // just in case
           rv = NS_ERROR_ABORT;
@@ -682,7 +680,7 @@ nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow *aParent,
       }
       else
         rv = mWindowCreator->CreateChromeWindow(parentChrome, chromeFlags,
-                               getter_AddRefs(newChrome));
+                                                getter_AddRefs(newChrome));
       if (newChrome) {
         /* It might be a chrome nsXULWindow, in which case it won't have
             an nsIDOMWindow (primary content shell). But in that case, it'll
@@ -729,7 +727,7 @@ nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow *aParent,
   }
 
   if ((aDialog || windowIsModalContentDialog) && argv) {
-    // Set the args on the new object.
+    // Set the args on the new window.
     nsCOMPtr<nsIScriptGlobalObject> scriptGlobal(do_QueryInterface(*_retval));
     NS_ENSURE_TRUE(scriptGlobal, NS_ERROR_UNEXPECTED);
     rv = scriptGlobal->SetNewArguments(argv);
@@ -912,9 +910,37 @@ nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow *aParent,
     nsCOMPtr<nsIDocShellTreeOwner> newTreeOwner;
     newDocShellItem->GetTreeOwner(getter_AddRefs(newTreeOwner));
     nsCOMPtr<nsIWebBrowserChrome> newChrome(do_GetInterface(newTreeOwner));
-    if (newChrome)
-      newChrome->ShowAsModal();
-    NS_ASSERTION(newChrome, "show modal window failed: no available chrome");
+
+    // Throw an exception here if no web browser chrome is available,
+    // we need that to show a modal window.
+    NS_ENSURE_TRUE(newChrome, NS_ERROR_NOT_AVAILABLE);
+
+    nsCOMPtr<nsPIDOMWindow> modalContentWindow;
+
+    // Dispatch dialog events etc, but we only want to do that if
+    // we're opening a modal content window (the helper classes are
+    // no-ops if given no window), for chrome dialogs we don't want to
+    // do any of that (it's done elsewhere for us).
+
+    if (windowIsModalContentDialog) {
+      modalContentWindow = do_QueryInterface(*_retval);
+    }
+
+    nsAutoWindowStateHelper windowStateHelper(modalContentWindow);
+
+    if (!windowStateHelper.DefaultEnabled()) {
+      // Default to cancel not opening the modal window.
+      NS_RELEASE(*_retval);
+
+      return NS_OK;
+    }
+
+    // Reset popup state while opening a modal dialog, and firing
+    // events about the dialog, to prevent the current state from
+    // being active the whole time a modal dialog is open.
+    nsAutoPopupStatePusher popupStatePusher(modalContentWindow, openAbused);
+
+    newChrome->ShowAsModal();
   }
 
   return NS_OK;

xpfe/appshell/src/nsXULWindow.cpp

@@ -1798,18 +1798,6 @@ NS_IMETHODIMP nsXULWindow::CreateNewContentWindow(PRInt32 aChromeFlags,
                                    (static_cast<nsIXULWindow*>
                                                (newWindow));
 
-  nsCOMPtr<nsIDocShell> newDocShell;
-  xulWin->GetDocShell(getter_AddRefs(newDocShell));
-
-  // If we're opening a non-chrome modal window (i.e. a modal content
-  // window), tell the DOM window that it is modal.
-  nsCOMPtr<nsPIDOMWindow> domWin(do_GetInterface(newDocShell));
-
-  if (domWin && (aChromeFlags & nsIWebBrowserChrome::CHROME_MODAL) &&
-      !(aChromeFlags & nsIWebBrowserChrome::CHROME_OPENAS_CHROME)) {
-    domWin->SetModalContentWindow(PR_TRUE);
-  }
-
   xulWin->LockUntilChromeLoad();
 
   // Push nsnull onto the JSContext stack before we dispatch a native event.