commit 4e61329ac0c2ce9b66733e8d65d3c8d3c02b9dcb
parent 74bc43245c32f0349182dea50406446df875ad7a
Author: Yoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Date: Wed, 8 Oct 2025 08:35:27 +0000
Bug 1992588 - Align error handling with the HTML spec when loading imported module scripts. r=jonco,frontend-codestyle-reviewers
According to the spec, a ThrowCompletion should be performed if the module
script is null or has a parse error.
See https://html.spec.whatwg.org/#hostloadimportedmodule:fetch-a-single-imported-module-script
Step 14.
So this patch reverts ModuleLoadRequest::IsErrored() update in https://phabricator.services.mozilla.com/D264807
Additionally, a spec issue has been filed to clarify whether an error to
rethrow should also be treated as a ThrowCompletion:
https://github.com/whatwg/html/issues/11755
Differential Revision: https://phabricator.services.mozilla.com/D267603
Diffstat:
12 files changed, 86 insertions(+), 8 deletions(-)
diff --git a/.prettierignore b/.prettierignore
@@ -1119,6 +1119,7 @@ dom/base/test/jsmodules/test_syntaxErrorAsync.html
dom/base/test/jsmodules/module_badSyntax.mjs
dom/base/test/jsmodules/test_syntaxErrorInline.html
dom/base/test/jsmodules/test_syntaxErrorInlineAsync.html
+dom/base/test/jsmodules/parse_error.js
dom/base/test/test_bug687859.html
dom/media/webrtc/tests/mochitests/identity/idp-bad.js
dom/security/test/general/file_nonscript.json
diff --git a/dom/base/test/jsmodules/chrome.toml b/dom/base/test/jsmodules/chrome.toml
@@ -10,6 +10,10 @@ support-files = [
"import_circular_1.mjs",
"import_no_export.mjs",
"import_no_indirect_export.mjs",
+ "import_parse_error.mjs",
+ "import_parse_error_1.mjs",
+ "import_parse_error_2.mjs",
+ "import_parse_error_3.mjs",
"exportA1.mjs",
"exportA2.mjs",
"export_ambiguous.mjs",
@@ -35,6 +39,7 @@ support-files = [
"module_multiLargeImports.mjs",
"no_export.mjs",
"no_indirect_export.mjs",
+ "parse_error.js",
"script_simple2.js",
"module_large1.mjs",
"module_large2.mjs",
@@ -48,9 +53,13 @@ support-files = [
["test_cyclicImport.html"]
+["test_dynamicImportErrorMessage.html"]
+
["test_dynamicImport_link_failure.html"]
-["test_dynamicImportErrorMessage.html"]
+["test_fetchedModuleHasErrorToRethrow.html"]
+
+["test_fetchedModuleHasErrorToRethrow_dynamic.html"]
["test_importIntroType.html"]
diff --git a/dom/base/test/jsmodules/import_parse_error.mjs b/dom/base/test/jsmodules/import_parse_error.mjs
@@ -0,0 +1,2 @@
+/* eslint-disable import/named */
+import { foo } from "./parse_error.js";
diff --git a/dom/base/test/jsmodules/import_parse_error_1.mjs b/dom/base/test/jsmodules/import_parse_error_1.mjs
@@ -0,0 +1,2 @@
+/* eslint-disable import/named */
+import { foo } from "./import_parse_error_2.mjs";
diff --git a/dom/base/test/jsmodules/import_parse_error_2.mjs b/dom/base/test/jsmodules/import_parse_error_2.mjs
@@ -0,0 +1,2 @@
+/* eslint-disable import/named */
+import { foo } from "./import_parse_error_3.mjs";
diff --git a/dom/base/test/jsmodules/import_parse_error_3.mjs b/dom/base/test/jsmodules/import_parse_error_3.mjs
@@ -0,0 +1,2 @@
+/* eslint-disable import/named */
+import { foo } from "./import_parse_error.mjs";
diff --git a/dom/base/test/jsmodules/parse_error.js b/dom/base/test/jsmodules/parse_error.js
@@ -0,0 +1 @@
+const foo;=1
diff --git a/dom/base/test/jsmodules/test_fetchedModuleHasErrorToRethrow.html b/dom/base/test/jsmodules/test_fetchedModuleHasErrorToRethrow.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Test an imported module has an error to rethrow</title>
+<script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+<script>
+ SimpleTest.waitForExplicitFinish();
+
+ let count = 0;
+ let hasErrorToRethrow = false;
+ window.onerror = (e) => {
+ hasErrorToRethrow = true;
+ count++;
+ };
+
+ function testLoaded() {
+ is(hasErrorToRethrow, true, "hasErrorToRethrow should be set to true");
+ is(count, 2, "window.onerror should be called twice");
+ SimpleTest.finish();
+ }
+</script>
+<script type="module" src="./import_parse_error.mjs"></script>
+<script type="module" src="./import_parse_error_1.mjs"></script>
+<body onload='testLoaded()'></body>
diff --git a/dom/base/test/jsmodules/test_fetchedModuleHasErrorToRethrow_dynamic.html b/dom/base/test/jsmodules/test_fetchedModuleHasErrorToRethrow_dynamic.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Test an imported module has an error to rethrow when import()</title>
+<script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+<script type="module" src="./import_parse_error.mjs"></script>
+<script>
+ SimpleTest.waitForExplicitFinish();
+
+ let hasErrorToRethrow = false;
+ window.onerror = (e) => {
+ hasErrorToRethrow = true;
+ };
+
+ function testLoaded() {
+ is(hasErrorToRethrow, true, "hasErrorToRethrow should be set to true");
+ import("./import_parse_error_3.mjs").then(() => {
+ ok(false, "Should have thrown SyntaxError");
+ }).catch((e) => {
+ ok(e instanceof SyntaxError , "Should throw a SyntaxError");
+ SimpleTest.finish();
+ });
+ }
+</script>
+<body onload='testLoaded()'></body>
diff --git a/eslint-ignores.config.mjs b/eslint-ignores.config.mjs
@@ -159,6 +159,7 @@ export default [
"dom/base/test/jsmodules/module_badSyntax.mjs",
"dom/base/test/jsmodules/test_syntaxErrorInline.html",
"dom/base/test/jsmodules/test_syntaxErrorInlineAsync.html",
+ "dom/base/test/jsmodules/parse_error.js",
"dom/base/test/test_bug687859.html",
"dom/media/webrtc/tests/mochitests/identity/idp-bad.js",
"dom/security/test/general/file_nonscript.json",
diff --git a/js/loader/ModuleLoadRequest.cpp b/js/loader/ModuleLoadRequest.cpp
@@ -74,12 +74,7 @@ nsIGlobalObject* ModuleLoadRequest::GetGlobalObject() {
}
bool ModuleLoadRequest::IsErrored() const {
- if (!mModuleScript || mErroredLoadingImports) {
- return true;
- }
-
- MOZ_ASSERT_IF(mModuleScript->HasErrorToRethrow(), !IsDynamicImport());
- return mModuleScript->HasParseError() || mModuleScript->HasErrorToRethrow();
+ return !mModuleScript || mModuleScript->HasParseError();
}
void ModuleLoadRequest::SetReady() {
@@ -134,7 +129,14 @@ void ModuleLoadRequest::ModuleErrored() {
}
MOZ_ASSERT(!IsFinished());
- MOZ_ASSERT(IsErrored());
+
+ mozilla::DebugOnly<bool> hasRethrow =
+ mModuleScript && mModuleScript->HasErrorToRethrow();
+ MOZ_ASSERT_IF(hasRethrow, !IsDynamicImport());
+
+ // When LoadRequestedModules fails, we will set error to rethrow to the module
+ // script or call SetErroredLoadingImports() and then call ModuleErrored().
+ MOZ_ASSERT(IsErrored() || hasRethrow || mErroredLoadingImports);
if (IsFinished()) {
// Cancelling an outstanding import will error this request.
diff --git a/js/loader/ModuleLoaderBase.cpp b/js/loader/ModuleLoaderBase.cpp
@@ -770,6 +770,15 @@ nsresult ModuleLoaderBase::OnFetchComplete(ModuleLoadRequest* aRequest,
bool success = bool(aRequest->mModuleScript);
MOZ_ASSERT(NS_SUCCEEDED(rv) == success);
+ // TODO: https://github.com/whatwg/html/issues/11755
+ // Should we check if the module script has an error to rethrow as well?
+ //
+ // https://html.spec.whatwg.org/#hostloadimportedmodule:fetch-a-single-imported-module-script
+ // Step 14. onSingleFetchComplete: step 2, and step 3
+ // return ThrowCompletion if moduleScript is null or its parse error is not
+ // null:
+ // Step 4. Otherwise, set completion to NormalCompletion(moduleScript's
+ // record).
if (!aRequest->IsErrored()) {
OnFetchSucceeded(aRequest);
} else {
