Bug 531176. Patch by Olli Pettay <Olli.Pettay@helsinki.fi>, r=jonas, sr=jst, a1.9.0.next=dveditz

git-svn-id: svn://10.0.0.236/trunk@260868 18797224-902f-48f8-a5cc-f745e15eee43

mozilla/dom/src/base/nsGlobalWindow.cpp

@@ -6156,6 +6156,22 @@ nsGlobalWindow::ShowModalDialog(const nsAString& aURI, nsIVariant *aArgs,
   return NS_OK;
 }
 
+class CommandDispatcher : public nsRunnable
+{
+public:
+  CommandDispatcher(nsIDOMXULCommandDispatcher* aDispatcher,
+                    const nsAString& aAction)
+  : mDispatcher(aDispatcher), mAction(aAction) {}
+
+  NS_IMETHOD Run()
+  {
+    return mDispatcher->UpdateCommands(mAction);
+  }
+
+  nsCOMPtr<nsIDOMXULCommandDispatcher> mDispatcher;
+  nsString                             mAction;
+};
+
 NS_IMETHODIMP
 nsGlobalWindow::UpdateCommands(const nsAString& anAction)
 {
@@ -6170,7 +6186,10 @@ nsGlobalWindow::UpdateCommands(const nsAString& anAction)
     // Retrieve the command dispatcher and call updateCommands on it.
     nsCOMPtr<nsIDOMXULCommandDispatcher> xulCommandDispatcher;
     xulDoc->GetCommandDispatcher(getter_AddRefs(xulCommandDispatcher));
-    xulCommandDispatcher->UpdateCommands(anAction);
+    if (xulCommandDispatcher) {
+      nsContentUtils::AddScriptRunner(new CommandDispatcher(xulCommandDispatcher,
+                                                            anAction));
+    }
   }
 
   return NS_OK;

mozilla/dom/src/base/nsJSEnvironment.cpp

@@ -420,109 +420,73 @@ NS_HandleScriptError(nsIScriptGlobalObject *aScriptGlobal,
   return called;
 }
 
-// NOTE: This function could be refactored to use the above.  The only reason
+class ScriptErrorEvent : public nsRunnable
-// it has not been done is that the code below only fills the error event
-// after it has a good nsPresContext - whereas using the above function
-// would involve always filling it.  Is that a concern?
-void JS_DLL_CALLBACK
-NS_ScriptErrorReporter(JSContext *cx,
-                       const char *message,
-                       JSErrorReport *report)
 {
-  // XXX this means we are not going to get error reports on non DOM contexts
+public:
-  nsIScriptContext *context = nsJSUtils::GetDynamicScriptContext(cx);
+  ScriptErrorEvent(nsIScriptGlobalObject* aScriptGlobal,
+                   PRUint32 aLineNr, PRUint32 aColumn, PRUint32 aFlags,
+                   const nsAString& aErrorMsg,
+                   const nsAString& aFileName,
+                   const nsAString& aSourceLine,
+                   PRBool aDispatchEvent)
+  : mScriptGlobal(aScriptGlobal), mLineNr(aLineNr), mColumn(aColumn),
+    mFlags(aFlags), mErrorMsg(aErrorMsg), mFileName(aFileName),
+    mSourceLine(aSourceLine), mDispatchEvent(aDispatchEvent) {}
 
-  nsEventStatus status = nsEventStatus_eIgnore;
+  NS_IMETHOD Run()
-
-  // Note: we must do this before running any more code on cx (if cx is the
-  // dynamic script context).
-  ::JS_ClearPendingException(cx);
-
-  if (context) {
-    nsIScriptGlobalObject *globalObject = context->GetGlobalObject();
-
-    if (globalObject) {
-      nsAutoString fileName, msg;
-      NS_NAMED_LITERAL_STRING(xoriginMsg, "Script error.");
-
-      fileName.AssignWithConversion(report->filename);
-
-      const PRUnichar *m = reinterpret_cast<const PRUnichar*>
-                                             (report->ucmessage);
-      if (m) {
-        msg.Assign(m);
-      }
-
-      if (msg.IsEmpty() && message) {
-        msg.AssignWithConversion(message);
-      }
-
-      // First, notify the DOM that we have a script error.
-      /* We do not try to report Out Of Memory via a dom
-       * event because the dom event handler would encounter
-       * an OOM exception trying to process the event, and
-       * then we'd need to generate a new OOM event for that
-       * new OOM instance -- this isn't pretty.
-       */
   {
-        // Scope to make sure we're not using |win| in the rest of
+    nsEventStatus status = nsEventStatus_eIgnore;
-        // this function when we should be using |globalObject|.  We
+    // First, notify the DOM that we have a script error.
-        // only need |win| for the event dispatch.
+    if (mDispatchEvent) {
-        nsCOMPtr<nsPIDOMWindow> win(do_QueryInterface(globalObject));
+      nsCOMPtr<nsPIDOMWindow> win(do_QueryInterface(mScriptGlobal));
-        nsIDocShell *docShell = win ? win->GetDocShell() : nsnull;
+      nsIDocShell* docShell = win ? win->GetDocShell() : nsnull;
       if (docShell &&
-            (report->errorNumber != JSMSG_OUT_OF_MEMORY &&
+          !JSREPORT_IS_WARNING(mFlags) &&
-              !JSREPORT_IS_WARNING(report->flags))) {
+          !sHandlingScriptError) {
-          static PRInt32 errorDepth; // Recursion prevention
+        sHandlingScriptError = PR_TRUE; // Recursion prevention
-          ++errorDepth;
 
         nsCOMPtr<nsPresContext> presContext;
         docShell->GetPresContext(getter_AddRefs(presContext));
 
-          if (presContext && errorDepth < 2) {
+        if (presContext) {
           nsScriptErrorEvent errorevent(PR_TRUE, NS_LOAD_ERROR);
 
-            errorevent.fileName = fileName.get();
+          errorevent.fileName = mFileName.get();
 
           nsCOMPtr<nsIScriptObjectPrincipal> sop(do_QueryInterface(win));
-            nsIPrincipal *p = sop->GetPrincipal();
+          NS_ENSURE_STATE(sop);
+          nsIPrincipal* p = sop->GetPrincipal();
+          NS_ENSURE_STATE(p);
 
-            PRBool sameOrigin = (report->filename == nsnull);
+          PRBool sameOrigin = mFileName.IsVoid();
 
           if (p && !sameOrigin) {
             nsCOMPtr<nsIURI> errorURI;
-              NS_NewURI(getter_AddRefs(errorURI), report->filename);
+            NS_NewURI(getter_AddRefs(errorURI), mFileName);
-
+            if (errorURI) {
-              nsCOMPtr<nsIURI> codebase;
-              p->GetURI(getter_AddRefs(codebase));
-
-              if (errorURI && codebase) {
               // FIXME: Once error reports contain the origin of the
               // error (principals) we should change this to do the
               // security check based on the principals and not
               // URIs. See bug 387476.
-                sameOrigin =
+              sameOrigin = NS_SUCCEEDED(p->CheckMayLoad(errorURI, PR_FALSE));
-                  NS_SUCCEEDED(sSecurityManager->
-                               CheckSameOriginURI(errorURI, codebase,
-                                                  PR_FALSE));
             }
           }
 
+          NS_NAMED_LITERAL_STRING(xoriginMsg, "Script error.");
           if (sameOrigin) {
-              errorevent.errorMsg = msg.get();
+            errorevent.errorMsg = mErrorMsg.get();
-              errorevent.lineNr = report->lineno;
+            errorevent.lineNr = mLineNr;
           } else {
+            NS_WARNING("Not same origin error!");
             errorevent.errorMsg = xoriginMsg.get();
             errorevent.lineNr = 0;
           }
 
-            // Dispatch() must be synchronous for the recursion block
-            // (errorDepth) to work.
           nsEventDispatcher::Dispatch(win, presContext, &errorevent, nsnull,
                                       &status);
         }
 
-          --errorDepth;
+        sHandlingScriptError = PR_FALSE;
       }
     }
 
@@ -537,7 +501,7 @@ NS_ScriptErrorReporter(JSContext *cx,
 
         // Set category to chrome or content
         nsCOMPtr<nsIScriptObjectPrincipal> scriptPrincipal =
-            do_QueryInterface(globalObject);
+          do_QueryInterface(mScriptGlobal);
         NS_ASSERTION(scriptPrincipal, "Global objects must implement "
                      "nsIScriptObjectPrincipal");
         nsCOMPtr<nsIPrincipal> systemPrincipal;
@@ -547,12 +511,9 @@ NS_ScriptErrorReporter(JSContext *cx,
           ? "chrome javascript"
           : "content javascript";
 
-          PRUint32 column = report->uctokenptr - report->uclinebuf;
+        rv = errorObject->Init(mErrorMsg.get(), mFileName.get(),
-
+                               mSourceLine.get(),
-          rv = errorObject->Init(msg.get(), fileName.get(),
+                               mLineNr, mColumn, mFlags,
-                                 reinterpret_cast<const PRUnichar*>
-                                                 (report->uclinebuf),
-                                 report->lineno, column, report->flags,
                                category);
 
         if (NS_SUCCEEDED(rv)) {
@@ -564,6 +525,75 @@ NS_ScriptErrorReporter(JSContext *cx,
         }
       }
     }
+    return NS_OK;
+  }
+
+
+  nsCOMPtr<nsIScriptGlobalObject> mScriptGlobal;
+  PRUint32                        mLineNr;
+  PRUint32                        mColumn;
+  PRUint32                        mFlags;
+  nsString                        mErrorMsg;
+  nsString                        mFileName;
+  nsString                        mSourceLine;
+  PRBool                          mDispatchEvent;
+
+  static PRBool sHandlingScriptError;
+};
+
+PRBool ScriptErrorEvent::sHandlingScriptError = PR_FALSE;
+
+// NOTE: This function could be refactored to use the above.  The only reason
+// it has not been done is that the code below only fills the error event
+// after it has a good nsPresContext - whereas using the above function
+// would involve always filling it.  Is that a concern?
+void JS_DLL_CALLBACK
+NS_ScriptErrorReporter(JSContext *cx,
+                       const char *message,
+                       JSErrorReport *report)
+{
+  // XXX this means we are not going to get error reports on non DOM contexts
+  nsIScriptContext *context = nsJSUtils::GetDynamicScriptContext(cx);
+
+  // Note: we must do this before running any more code on cx (if cx is the
+  // dynamic script context).
+  ::JS_ClearPendingException(cx);
+
+  if (context) {
+    nsIScriptGlobalObject *globalObject = context->GetGlobalObject();
+
+    if (globalObject) {
+      nsAutoString fileName, msg;
+      if (!report->filename) {
+        fileName.SetIsVoid(PR_TRUE);
+      } else {
+        fileName.AssignWithConversion(report->filename);
+      }
+
+      const PRUnichar *m = reinterpret_cast<const PRUnichar*>
+                                             (report->ucmessage);
+      if (m) {
+        msg.Assign(m);
+      }
+
+      if (msg.IsEmpty() && message) {
+        msg.AssignWithConversion(message);
+      }
+
+
+      /* We do not try to report Out Of Memory via a dom
+       * event because the dom event handler would encounter
+       * an OOM exception trying to process the event, and
+       * then we'd need to generate a new OOM event for that
+       * new OOM instance -- this isn't pretty.
+       */
+      nsAutoString sourceLine;
+      sourceLine.Assign(reinterpret_cast<const PRUnichar*>(report->uclinebuf));
+      nsContentUtils::AddScriptRunner(
+        new ScriptErrorEvent(globalObject, report->lineno,
+                             report->uctokenptr - report->uclinebuf,
+                             report->flags, msg, fileName, sourceLine,
+                             report->errorNumber != JSMSG_OUT_OF_MEMORY));
     }
   }
 
@@ -588,8 +618,7 @@ NS_ScriptErrorReporter(JSContext *cx,
   } else {
     error.Append(message);
   }
-  if (status != nsEventStatus_eIgnore && !JSREPORT_IS_WARNING(report->flags))
+
-    error.Append(" Error was suppressed by event handler\n");
   fprintf(stderr, "%s\n", error.get());
   fflush(stderr);
 #endif

mozilla/dom/tests/mochitest/bugs/Makefile.in

@@ -81,6 +81,7 @@ _TEST_FILES	= \
 		iframe_bug440572.html \
 		test_bug504862.html \
 		file_bug504862.html \
+		test_bug531176.html \
 		$(NULL)
 
 libs:: 	$(_TEST_FILES)

mozilla/dom/tests/mochitest/bugs/test_bug531176.html

@@ -0,0 +1,61 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=531176
+-->
+<head>
+  <title>Test for Bug 531176</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=531176">Mozilla Bug 531176</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 531176 **/
+
+var errorCount = 0;
+function errorHandler(msg, filename, linenr) {
+  is(msg, "syntax error", "Wrong error!");
+  is(filename, window.location, "Wrong filename!");
+  is(linenr, 1, "Wrong linenr!");
+  ++errorCount;
+}
+
+window.onerror = errorHandler;
+document.body.setAttribute("onclick", "var x=;");
+is(errorCount, 1, "Error handler should have been called! (1)");
+
+function recursiveHandler(msg, filename, linenr) {
+  is(msg, "syntax error", "Wrong error!");
+  is(filename, window.location, "Wrong filename!");
+  is(linenr, 1, "Wrong linenr!");
+  ++errorCount;
+  document.body.setAttribute("onclick", "var z=;");
+}
+
+window.onerror = recursiveHandler;
+document.body.setAttribute("onclick", "var y=;");
+is(errorCount, 2, "Error handler should have been called! (2)");
+
+// Check that error handler works even after recursion error.
+document.body.setAttribute("onclick", "var foo=;");
+is(errorCount, 3, "Error handler should have been called! (3)");
+
+window.onerror = function() { ++errorCount; };
+</script>
+<script>
+var foo =;
+</script>
+<script>
+is(errorCount, 4, "Error handler should have been called! (4)");
+</script>
+</pre>
+</body>
+</html>

mozilla/layout/forms/nsComboboxControlFrame.cpp

@@ -90,6 +90,7 @@
 #include "nsDisplayList.h"
 #include "nsITheme.h"
 #include "nsThemeConstants.h"
+#include "nsPLDOMEvent.h"
 
 NS_IMETHODIMP
 nsComboboxControlFrame::RedisplayTextEvent::Run()
@@ -1485,18 +1486,9 @@ nsComboboxControlFrame::OnOptionSelected(nsPresContext* aPresContext,
 void nsComboboxControlFrame::FireValueChangeEvent()
 {
   // Fire ValueChange event to indicate data value of combo box has changed
-  nsCOMPtr<nsIDOMEvent> event;
+  nsCOMPtr<nsIDOMNode> node = do_QueryInterface(mContent);
-  nsPresContext* presContext = PresContext();
+  nsContentUtils::AddScriptRunner(new nsPLDOMEvent(node,
-  if (NS_SUCCEEDED(nsEventDispatcher::CreateEvent(presContext, nsnull,
+                                  NS_LITERAL_STRING("ValueChange")));
-                                                  NS_LITERAL_STRING("Events"),
-                                                  getter_AddRefs(event)))) {
-    event->InitEvent(NS_LITERAL_STRING("ValueChange"), PR_TRUE, PR_TRUE);
-
-    nsCOMPtr<nsIPrivateDOMEvent> privateEvent(do_QueryInterface(event));
-    privateEvent->SetTrusted(PR_TRUE);
-    nsEventDispatcher::DispatchDOMEvent(mContent, nsnull, event, nsnull,
-                                        nsnull);
-  }
 }
 
 void