Bug 437361. Propagate exceptions from showModalDialog's guts to script as needed instead of dropping them on the floor. r+sr=bzbarsky
authorBen Newman <bnewman@mozilla.com>
Mon, 28 Jul 2008 22:52:53 -0700
changeset 16266 7e2bdc442f1a6aa2c0957e29201d3bf69803114f
parent 16265 b6b0f1c44dc9e1a5d64164a22cd3e065603e1b3c
child 16267 c5b7201cf5c05c28c85632535a54eab7915d79bb
push idunknown
push userunknown
push dateunknown
bugs437361
milestone1.9.1a2pre
Bug 437361. Propagate exceptions from showModalDialog's guts to script as needed instead of dropping them on the floor. r+sr=bzbarsky
dom/src/base/nsGlobalWindow.cpp
dom/tests/mochitest/bugs/Makefile.in
dom/tests/mochitest/bugs/test_bug437361.html
testing/mochitest/Makefile.in
testing/mochitest/mozprefs.js
--- a/dom/src/base/nsGlobalWindow.cpp
+++ b/dom/src/base/nsGlobalWindow.cpp
@@ -6045,34 +6045,36 @@ nsGlobalWindow::ShowModalDialog(const ns
   // Before bringing up the window, unsuppress painting and flush
   // pending reflows.
   EnsureReflowFlushAndPaint();
 
   nsresult rv = OpenInternal(aURI, EmptyString(), options,
                              PR_FALSE,          // aDialog
                              PR_TRUE,           // aContentModal
                              PR_TRUE,           // aCalledNoScript
-                             PR_FALSE,          // aDoJSFixups
+                             PR_TRUE,           // aDoJSFixups
                              nsnull, aArgs,     // args
                              GetPrincipal(),    // aCalleePrincipal
                              nsnull,            // aJSCallerContext
                              getter_AddRefs(dlgWin));
-  if (NS_FAILED(rv) || !dlgWin)
-    return NS_OK;
-
-  nsCOMPtr<nsPIDOMWindow> win(do_QueryInterface(dlgWin));
-
-  nsPIDOMWindow *inner = win->GetCurrentInnerWindow();
-
-  nsCOMPtr<nsIDOMModalContentWindow> dlgInner(do_QueryInterface(inner));
-
-  if (dlgInner) {
-    dlgInner->GetReturnValue(aRetVal);
-  }
-
+
+  NS_ENSURE_SUCCESS(rv, rv);
+  
+  if (dlgWin) {
+    nsCOMPtr<nsPIDOMWindow> win(do_QueryInterface(dlgWin));
+
+    nsPIDOMWindow *inner = win->GetCurrentInnerWindow();
+
+    nsCOMPtr<nsIDOMModalContentWindow> dlgInner(do_QueryInterface(inner));
+
+    if (dlgInner) {
+      dlgInner->GetReturnValue(aRetVal);
+    }
+  }
+  
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsGlobalWindow::UpdateCommands(const nsAString& anAction)
 {
   nsPIDOMWindow *rootWindow = nsGlobalWindow::GetPrivateRoot();
   if (!rootWindow)
@@ -7287,18 +7289,16 @@ nsGlobalWindow::OpenInternal(const nsASt
   PRUint32 argc = 0;
   if (argv)
       argv->GetLength(&argc);
 #endif
   NS_PRECONDITION(!aExtraArgument || (!argv && argc == 0),
                   "Can't pass in arguments both ways");
   NS_PRECONDITION(!aCalledNoScript || (!argv && argc == 0),
                   "Can't pass JS args when called via the noscript methods");
-  NS_PRECONDITION(!aDoJSFixups || !aCalledNoScript,
-                  "JS fixups should not be done when called noscript");
   NS_PRECONDITION(!aJSCallerContext || !aCalledNoScript,
                   "Shouldn't have caller context when called noscript");
 
   *aReturn = nsnull;
   
   nsCOMPtr<nsIWebBrowserChrome> chrome;
   GetWebBrowserChrome(getter_AddRefs(chrome));
   if (!chrome) {
--- a/dom/tests/mochitest/bugs/Makefile.in
+++ b/dom/tests/mochitest/bugs/Makefile.in
@@ -74,12 +74,13 @@ include $(topsrcdir)/config/rules.mk
 		iframe_bug409349.html \
 		test_bug411103.html \
 		test_bug414291.html \
 		test_bug430276.html \
 		iframe_bug430276.html \
 		iframe_bug430276-2.html \
 		test_bug440572.html \
 		iframe_bug440572.html \
+		test_bug437361.html \
 		$(NULL)
 
 libs:: 	$(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/dom/tests/mochitest/bugs/test_bug437361.html
@@ -0,0 +1,67 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=437361
+-->
+<head>
+  <title>Test for Bug 437361</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/mozprefs.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  
+  <script class="testbody" type="text/javascript">
+
+  /** Test for Bug 437361 **/
+
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+
+  function testModalDialogBlockedCleanly() {
+    is(true, pref("dom.disable_open_during_load"), "mozprefs sanity check");
+    var rv = window.showModalDialog( // should be blocked without exception
+      "data:text/html,<html><body onload='close(); returnValue = 1;' /></html>");
+    is(rv, null, "Modal dialog opened unexpectedly.");
+  }
+  
+  function testModalDialogAllowed() {
+    is(false, pref("dom.disable_open_during_load"), "mozprefs sanity check");
+    var rv = window.showModalDialog( // should not be blocked this time
+      "data:text/html,<html><body onload='close(); returnValue = 1;' /></html>");
+    is(rv, 1, "Problem with modal dialog returnValue.");
+  }
+
+  function testOtherExceptionsNotTrapped() {
+    is(false, pref("dom.disable_open_during_load"), "mozprefs sanity check");
+    window.showModalDialog('about:config'); // forbidden by SecurityCheckURL
+  }
+
+  function test(disableOpen, exceptionExpected, testFn, errorMsg) {
+    try {
+      pref("dom.disable_open_during_load", disableOpen, testFn);
+      ok(!exceptionExpected, errorMsg);
+    } catch (_) {
+      ok(exceptionExpected, errorMsg);
+    }
+  }
+
+  test(true, false, testModalDialogBlockedCleanly,
+       "Blocked showModalDialog caused an exception.");
+       
+  test(false, false, testModalDialogAllowed,
+       "showModalDialog was blocked even though dom.disable_open_during_load was false.");
+
+  test(false, true, testOtherExceptionsNotTrapped,
+       "Incorrectly suppressed insecure showModalDialog exception.");
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=437361">Mozilla Bug 437361</a>
+<p id="display"></p>
+<div id="content" style="display: none"> 
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>
+
--- a/testing/mochitest/Makefile.in
+++ b/testing/mochitest/Makefile.in
@@ -64,16 +64,17 @@ include $(topsrcdir)/config/rules.mk
 		browser-test-overlay.xul \
 		browser-test.js \
 		browser-harness.xul \
 		redirect-a11y.html \
 		redirect.html \
 		redirect.js \
 		$(topsrcdir)/build/pgo/server-locations.txt \
 		$(topsrcdir)/netwerk/test/httpserver/httpd.js \
+		mozprefs.js \
 		$(NULL)	
 
 
 _DEST_DIR = $(DEPTH)/_tests/$(relativesrcdir)
 
 ifeq ($(USE_SHORT_LIBNAME), 1)
 PROGRAM = $(MOZ_APP_NAME)$(BIN_SUFFIX)
 else
new file mode 100644
--- /dev/null
+++ b/testing/mochitest/mozprefs.js
@@ -0,0 +1,92 @@
+(function() {
+
+  // NOTE: You *must* also include this line in any test that uses this file:
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+  
+  var prefService = Components.classes["@mozilla.org/preferences-service;1"]
+                              .getService(Components.interfaces.nsIPrefService);
+
+  function determinePrefKind(branch, prefName) {
+    switch (branch.getPrefType(prefName)) {
+    case branch.PREF_STRING:    return "CharPref";
+    case branch.PREF_INT:       return "IntPref";
+    case branch.PREF_BOOL:      return "BoolPref";
+    default: /* PREF_INVALID */ return "ComplexValue";
+    }
+  }
+  
+  function memoize(fn, obj) {
+    var cache = {}, sep = '___',
+        join = Array.prototype.join;
+    return function() {
+      var key = join.call(arguments, sep);
+      if (!(key in cache))
+        cache[key] = fn.apply(obj, arguments);
+      return cache[key];
+    };
+  }
+  
+  var makeAccessor = memoize(function(pref) {
+    var splat = pref.split('.'),
+        basePref = splat.pop(),
+        branch, kind;
+    
+    try {
+      branch = prefService.getBranch(splat.join('.') + '.')
+    } catch (e) {
+      alert("Calling prefService.getBranch failed: " + 
+        "did you read the NOTE in mozprefs.js?");
+      throw e;
+    }
+    
+    kind = determinePrefKind(branch, basePref);
+    
+    return function(value) {
+      var oldValue = branch['get' + kind](basePref);
+      if (arguments.length > 0)
+        branch['set' + kind](basePref, value);
+      return oldValue;
+    };
+  });
+
+  /* function pref(name[, value[, fn[, obj]]])
+   * -----------------------------------------
+   * Use cases:
+   *
+   *   1. Get the value of the dom.disable_open_during_load preference:
+   *
+   *      pref('dom.disable_open_during_load')
+   *
+   *   2. Set the preference to true, returning the old value:
+   *
+   *      var oldValue = pref('dom.disable_open_during_load', true);
+   *
+   *   3. Set the value of the preference to true just for the duration
+   *      of the specified function's execution:
+   *
+   *      pref('dom.disable_open_during_load', true, function() {
+   *        window.open(this.getUrl()); // fails if still loading
+   *      }, this); // for convenience, allow binding
+   *
+   *      Rationale: Unless a great deal of care is taken to catch all
+   *                 exceptions and restore original preference values,
+   *                 manually setting & restoring preferences can lead
+   *                 to unpredictable test behavior.  The try-finally
+   *                 block below eliminates that risk.
+   */
+  function pref(name, /*optional:*/ value, fn, obj) {
+    var acc = makeAccessor(name);
+    switch (arguments.length) {
+    case 1: return acc();
+    case 2: return acc(value);
+    default:
+      var oldValue = acc(value),
+          extra_args = [].slice.call(arguments, 4);
+      try { return fn.apply(obj, extra_args) }
+      finally { acc(oldValue) } // reset no matter what
+    }
+  };
+  
+  window.pref = pref; // export
+ 
+})();