JS module tests related to error handling currently fail

RESOLVED FIXED in Firefox 59

Status

()

defect
P3
normal
RESOLVED FIXED
a year ago
2 months ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

unspecified
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

a year ago
The following web platform tests currently fail:

html/semantics/scripting-1/the-script-element/module/load-error-events-inline.html
html/semantics/scripting-1/the-script-element/module/errorhandling.html
Assignee

Comment 1

a year ago
Factor out error handling from ScriptLoader::OnStreamComplete() into a new method.

I moved the test for NS_BINDING_RETARGETED to before the call to HandleError.  I don't think it's possible for rv to be NS_ERROR_TRACKING_URI while aChannelError is NS_BINDING_RETARGETED so this shouldn't matter.
Attachment #8937745 - Flags: review?(amarchesini)
Assignee

Comment 2

a year ago
While I was there I broke OnStreamComplete up a bit to make it easier to follow.
Attachment #8937746 - Flags: review?(amarchesini)
Assignee

Comment 3

a year ago
Now the fix.  Call ProcessFetchedModuleSource and HandleLoadError for inline module scripts and move the call to SetModuleFetchFinishedAndResumeWaitingRequests to HandleError for the error case so that it gets called for fetch errors that happen before ProcessFetchedModuleSource gets called.
Attachment #8937756 - Flags: review?(amarchesini)
Assignee

Updated

a year ago
Priority: -- → P3
Attachment #8937745 - Flags: review?(amarchesini) → review+
Comment on attachment 8937746 [details] [diff] [review]
bug1426140-2-refactor-onstreamcomplete

Review of attachment 8937746 [details] [diff] [review]:
-----------------------------------------------------------------

maybe some of those new methods can be const.

::: dom/script/ScriptLoader.cpp
@@ +2872,5 @@
>  }
>  
>  void
> +ScriptLoader::ReportErrorToConsole(ScriptLoadRequest *aRequest, nsresult aResult)
> +{

MOZ_ASSERT(aRequest);
Attachment #8937746 - Flags: review?(amarchesini) → review+
Attachment #8937756 - Flags: review?(amarchesini) → review+

Comment 5

a year ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b06fe374260
Factor out error handling from ScriptLoader::OnStreamComplete r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/54ba9609db75
Factor out SRI handling from ScriptLoader::OnStreamComplete r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8f97458d0cc
Handle errors for inline module scripts and ensure we update the module map after fetch errors r=baku

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2b06fe374260
https://hg.mozilla.org/mozilla-central/rev/54ba9609db75
https://hg.mozilla.org/mozilla-central/rev/a8f97458d0cc
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Assignee

Updated

a year ago
Duplicate of this bug: 1361373
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.