Bug 789856. Fire error events on <script> elements which completely fail to start the load. r=sicking
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 18 Sep 2012 23:24:27 -0400
changeset 107454 e61b7c193325f33b5a1f7238271632eb93982fc1
parent 107453 17dcbe563c2b00d5bb4544138c7e4355f3cbd62f
child 107455 09c03f14f5f50d17ca0dcc2994b1ad0898b34fb4
push id15045
push userbzbarsky@mozilla.com
push dateWed, 19 Sep 2012 03:25:45 +0000
treeherdermozilla-inbound@e61b7c193325 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs789856
milestone18.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 789856. Fire error events on <script> elements which completely fail to start the load. r=sicking
content/base/public/nsIScriptElement.h
content/base/src/nsScriptElement.cpp
content/base/src/nsScriptElement.h
content/base/src/nsScriptLoader.cpp
content/base/test/Makefile.in
content/base/test/test_bug606729.html
content/base/test/test_bug789856.html
content/html/content/test/test_bug371375.html
--- a/content/base/public/nsIScriptElement.h
+++ b/content/base/public/nsIScriptElement.h
@@ -12,18 +12,18 @@
 #include "nsIScriptLoaderObserver.h"
 #include "nsWeakPtr.h"
 #include "nsIParser.h"
 #include "nsContentCreatorFunctions.h"
 #include "nsIDOMHTMLScriptElement.h"
 #include "mozilla/CORSMode.h"
 
 #define NS_ISCRIPTELEMENT_IID \
-{ 0x24ab3ff2, 0xd75e, 0x4be4, \
-  { 0x8d, 0x50, 0xd6, 0x75, 0x31, 0x29, 0xab, 0x65 } }
+{ 0x491628bc, 0xce7c, 0x4db4, \
+ { 0x93, 0x3f, 0xce, 0x1b, 0x75, 0xee, 0x75, 0xce } }
 
 /**
  * Internal interface implemented by script elements
  */
 class nsIScriptElement : public nsIScriptLoaderObserver {
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_ISCRIPTELEMENT_IID)
 
@@ -230,16 +230,21 @@ public:
    * Get the CORS mode of the script element
    */
   virtual mozilla::CORSMode GetCORSMode() const
   {
     /* Default to no CORS */
     return mozilla::CORS_NONE;
   }
 
+  /**
+   * Fire an error event
+   */
+  virtual nsresult FireErrorEvent() = 0;
+
 protected:
   /**
    * Processes the script if it's in the document-tree and links to or
    * contains a script. Once it has been evaluated there is no way to make it
    * reevaluate the script, you'll have to create a new element. This also means
    * that when adding a src attribute to an element that already contains an
    * inline script, the script referenced by the src attribute will not be
    * loaded.
--- a/content/base/src/nsScriptElement.cpp
+++ b/content/base/src/nsScriptElement.cpp
@@ -20,27 +20,32 @@ using namespace mozilla::dom;
 NS_IMETHODIMP
 nsScriptElement::ScriptAvailable(nsresult aResult,
                                  nsIScriptElement *aElement,
                                  bool aIsInline,
                                  nsIURI *aURI,
                                  int32_t aLineNo)
 {
   if (!aIsInline && NS_FAILED(aResult)) {
-    nsCOMPtr<nsIContent> cont =
-      do_QueryInterface((nsIScriptElement*) this);
+    return FireErrorEvent();
+  }
+  return NS_OK;
+}
 
-    return nsContentUtils::DispatchTrustedEvent(cont->OwnerDoc(),
-                                                cont,
-                                                NS_LITERAL_STRING("error"),
-                                                false /* bubbles */,
-                                                false /* cancelable */);
-  }
+/* virtual */ nsresult
+nsScriptElement::FireErrorEvent()
+{
+  nsCOMPtr<nsIContent> cont =
+    do_QueryInterface((nsIScriptElement*) this);
 
-  return NS_OK;
+  return nsContentUtils::DispatchTrustedEvent(cont->OwnerDoc(),
+                                              cont,
+                                              NS_LITERAL_STRING("error"),
+                                              false /* bubbles */,
+                                              false /* cancelable */);
 }
 
 NS_IMETHODIMP
 nsScriptElement::ScriptEvaluated(nsresult aResult,
                                  nsIScriptElement *aElement,
                                  bool aIsInline)
 {
   nsresult rv = NS_OK;
--- a/content/base/src/nsScriptElement.h
+++ b/content/base/src/nsScriptElement.h
@@ -26,16 +26,18 @@ public:
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
   NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
 
   nsScriptElement(mozilla::dom::FromParser aFromParser)
     : nsIScriptElement(aFromParser)
   {
   }
 
+  virtual nsresult FireErrorEvent();
+
 protected:
   // Internal methods
 
   /**
    * Check if this element contains any script, linked or inline
    */
   virtual bool HasScriptContent() = 0;
 
--- a/content/base/src/nsScriptLoader.cpp
+++ b/content/base/src/nsScriptLoader.cpp
@@ -477,16 +477,20 @@ nsScriptLoader::ProcessScriptElement(nsI
 
   // Step 14. in the HTML5 spec
   nsresult rv = NS_OK;
   nsRefPtr<nsScriptLoadRequest> request;
   if (aElement->GetScriptExternal()) {
     // external script
     nsCOMPtr<nsIURI> scriptURI = aElement->GetScriptURI();
     if (!scriptURI) {
+      // Asynchronously report the failure to create a URI object
+      NS_DispatchToCurrentThread(
+        NS_NewRunnableMethod(aElement,
+                             &nsIScriptElement::FireErrorEvent));
       return false;
     }
     CORSMode ourCORSMode = aElement->GetCORSMode();
     nsTArray<PreloadInfo>::index_type i =
       mPreloads.IndexOf(scriptURI.get(), 0, PreloadURIComparator());
     if (i != nsTArray<PreloadInfo>::NoIndex) {
       // preloaded
       // note that a script-inserted script can steal a preload!
@@ -511,17 +515,23 @@ nsScriptLoader::ProcessScriptElement(nsI
 
     if (!request) {
       // no usable preload
       request = new nsScriptLoadRequest(aElement, version, ourCORSMode);
       request->mURI = scriptURI;
       request->mIsInline = false;
       request->mLoading = true;
       rv = StartLoad(request, type);
-      NS_ENSURE_SUCCESS(rv, false);
+      if (NS_FAILED(rv)) {
+        // Asynchronously report the load failure
+        NS_DispatchToCurrentThread(
+          NS_NewRunnableMethod(aElement,
+                               &nsIScriptElement::FireErrorEvent));
+        return false;
+      }
     }
 
     request->mJSVersion = version;
 
     if (aElement->GetScriptAsync()) {
       mAsyncRequests.AppendElement(request);
       if (!request->mLoading) {
         // The script is available already. Run it ASAP when the event
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -555,16 +555,17 @@ MOCHITEST_FILES_B = \
 		test_XHR_anon.html \
 		file_XHR_anon.sjs \
 		test_XHR_system.html \
 		test_XHR_parameters.html \
 		test_ipc_messagemanager_blob.html \
 		test_mixed_content_blocker.html \
 		file_mixed_content_main.html \
 		file_mixed_content_server.sjs \
+		test_bug789856.html \
 		$(NULL)
 
 MOCHITEST_CHROME_FILES =	\
 		test_bug357450.js \
 		$(NULL)
 
 MOCHITEST_FILES_PARTS = $(foreach s,A B,MOCHITEST_FILES_$(s))
 
--- a/content/base/test/test_bug606729.html
+++ b/content/base/test/test_bug606729.html
@@ -11,30 +11,42 @@ https://bugzilla.mozilla.org/show_bug.cg
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=606729">Mozilla Bug 606729</a>
 <p id="display"></p>
 <div id="content" style="display: none">
   
 </div>
 <pre id="test">
+<script>
+  SimpleTest.waitForExplicitFinish();
+  var events = 0;
+  var expectedEvents = 2;
+  function eventFired() {
+    ++events;
+    if (events == expectedEvents) {
+      SimpleTest.finish();
+    }
+  }
+</script>
 <script
   src="data:"
-  onerror="ok(false, 'Script with src=data: should not fire onerror.');"
-  onload="ok(false, 'Script with src=data: should not fire onload.');"
+  onerror="ok(true, 'Script with src=data: should fire onerror.');
+           eventFired();"
+  onload="ok(false, 'Script with src=data: should not fire onload.');
+          eventFired();"
 >
 ok(false, "Script with src=data: should not run textContent.");
 </script>
 <script
   src="bogus:"
-  onerror="ok(false, 'Script with src=bogus: should not fire onerror.');"
-  onload="ok(false, 'Script with src=bogus: should not fire onload.');"
+  onerror="ok(true, 'Script with src=bogus: should fire onerror.');
+           eventFired();"
+  onload="ok(false, 'Script with src=bogus: should not fire onload.');
+          eventFired();"
 >
 ok(false, "Script with src=bogus: should not run textContent.");
 </script>
-<script class="testbody" type="text/javascript">
-ok(true, "Obligatory succeeding test assertion.");
-</script>
 </pre>
 </body>
 </html>
 
 
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug789856.html
@@ -0,0 +1,42 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=789856
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 789856</title>
+  <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=789856">Mozilla Bug 789856</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 789856 **/
+SimpleTest.waitForExplicitFinish();
+
+var script = document.createElement("script");
+script.onload = function() {
+  ok(false, "This script should not load");
+  SimpleTest.finish();
+}
+script.onerror = function() {
+  ok(true, "This script should fail to load");
+  SimpleTest.finish();
+}
+// If neither one fires,  the test fails, as it should
+
+// Use a URL the test is not allowed to load
+script.src = "file:///tmp/"
+document.body.appendChild(script);
+
+</script>
+</pre>
+</body>
+</html>
--- a/content/html/content/test/test_bug371375.html
+++ b/content/html/content/test/test_bug371375.html
@@ -34,17 +34,17 @@ https://bugzilla.mozilla.org/show_bug.cg
   s2.onload = function() { load2Called = true; };
   s2.onerror = function(event) { error2Called = true; event.stopPropagation(); };
   s2.src = 'data:text/plain, var x = 1;'
   document.body.appendChild(s2);
 
   SimpleTest.waitForExplicitFinish();
   addLoadEvent(function() {
     is(load1Called, false, "Load handler should not be called");
-    is(error1Called, false, "Error handler should not be called");
+    is(error1Called, true, "Error handler should be called");
     is(load2Called, true, "Load handler for valid script should be called");
     is(error2Called, false,
        "Error handler for valid script should not be called");
     SimpleTest.finish();
   });
 </script>
 </body>
 </html>