XMLHttpRequest: implement the `on<event>` handler properties by bkaradzic-microsoft · Pull Request #221 · BabylonJS/JsRuntimeHost

bkaradzic-microsoft · 2026-08-06T23:34:29Z

Problem

XHR previously dispatched only to addEventListener handlers. Assignments such as request.onreadystatechange = fn silently created expandos that were never invoked, leaving callers waiting indefinitely. Successful requests also never raised load.

Changes

Handler properties and ordering. Implements onreadystatechange, onload, onerror, onloadend, and onabort. Both registration styles share one ordered listener list per event:

struct Listener
{
    std::string callbackKey;
    bool isEventHandler;
    bool active{true};
};
std::unordered_map<std::string, std::vector<std::shared_ptr<Listener>>> m_listeners;

Callbacks live in a JavaScript WeakMap keyed by the XHR, not permanently rooted native references. This preserves reusable handlers while allowing an unreachable XHR and its self-capturing callbacks to be collected. Dispatch snapshots stable listener records, checks removal and the current callback before each invocation, and holds that invocation's function in a short-lived Napi::FunctionReference.

Reassignment keeps the property's original position. xhr.onload = f and addEventListener("load", f) are independent registrations; duplicate addEventListener calls are ignored. removeEventListener does not remove property handlers. Primitive property assignments clear the handler; non-callable objects retain their identity and are skipped during dispatch.

Events and exceptions. Callbacks receive the XHR as this, with the correct type, target, and currentTarget. readystatechange receives an Event; terminal events receive a ProgressEvent. Missing constructors are installed without replacing existing ones. Event methods, dispatch-phase cleanup, and stopImmediatePropagation are supported. Listener exceptions are reported through the runtime's unhandled-exception path without preventing later listeners from running.

Abort and reuse. Active abort synchronously dispatches readystatechange, abort, and loadend, then returns to UNSENT. Abort before send is inert; abort after completion clears the response without events. Send-generation guards suppress stale continuations and preserve replacement requests started reentrantly from callbacks.

A canceled transport is detached before callbacks run, so response getters cannot read a buffer the old worker is still writing. Response data is exposed only after transport completion. Reopening preserves responseType; failed validation or backend Open leaves the previous response or active request unchanged. Concurrent sends and mutation of transport configuration during an active send are rejected.

Transport outcomes. Completed HTTP responses, including 404, raise load then loadend; transport failures raise error then loadend with status 0. The dependency is temporarily pinned to BabylonJS/UrlLib#38, which checks Android JNI read failures and Apple fixed-length identity-body truncation, clears partial responses, and publishes HTTP status only after the complete body is read. Completed HTTP error bodies use getErrorStream(). Repoint the pin upstream when that companion merges.

Regression coverage

The XHR JavaScript suite now contains 41 cases in Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts, including handler identity, ordering and mutation, reuse, failed open, response-type preservation, synchronous/reentrant abort, and typed events.

Native regressions cover self-capturing-handler collection on Chakra, exception/propagation behavior, complete versus truncated 200/404 responses, and response reads while a loopback server streams a body. Loopback tests run on POSIX platforms, including Android.

Local validation:

Platform	Result
Windows / Chakra	252 JavaScript cases passed; both native XHR tests passed
Linux / QuickJS / ThreadSanitizer	248 JavaScript cases and 26 native tests passed, without suppressions; final targeted XHR rerun also passed
Android API 36.1 x86_64 / QuickJS / NDK 29	250 JavaScript cases and 25 native tests passed; one existing logger test skipped

The Android truncated-body regression crashes under CheckJNI with the old UrlLib and passes with the new published dependency pin. Test bundles are generated from Source/Scripts by CMake/Gradle; the old Tests/UnitTests/dist instructions no longer apply.

Compatibility

Handler properties and the missing successful load event are additive. These are behavioral changes, not purely additive:

Completed non-2xx HTTP responses now raise load, not error; callers should inspect xhr.status.
Cancellation now raises synchronous abort and loadend, not a later transport error.
Duplicate listener registrations no longer throw.
In-flight response buffers are not exposed, and active transport configuration cannot be mutated.

Polyfills/XMLHttpRequest/Readme.md documents these semantics and the remaining minimal-polyfill limitations.

Copilot

Pull request overview

This PR updates the XMLHttpRequest polyfill to support DOM-style on<event> handler properties (e.g., onreadystatechange, onload) and ensures successful requests also raise the load event (in addition to loadend). It adds unit tests to prevent regressions where on<event> assignments silently did nothing.

Changes:

Added instance accessors for onreadystatechange, onload, onerror, onloadend, and onabort, stored separately from addEventListener handlers.
Updated event dispatch to invoke on<event> handlers in addition to addEventListener handlers, and to raise load on success.
Added regression tests validating on<event> semantics (invocation, readback/replace/clear, and interaction with addEventListener).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
Tests/UnitTests/Scripts/tests.ts	Adds regression tests covering `on<event>` handler properties and `load`/`loadend` behavior.
Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h	Introduces plumbing (event indices + storage) for `on<event>` handler properties.
Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp	Implements `on<event>` accessors, dispatches them from `RaiseEvent`, raises `load` on success, and clears stored handlers after completion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bkaradzic-microsoft · 2026-08-07T05:10:41Z

Thanks -- both comments addressed in c870d43.

On the non-callable setter (XMLHttpRequest.cpp:101): throwing a TypeError here would actually diverge from the DOM. EventHandler attributes are declared [LegacyTreatNonObjectAsNull] in WebIDL, so a non-callable assignment is coerced to null rather than rejected -- xhr.onload = 0 leaves xhr.onload === null in every browser, silently. The current clear-on-non-function behavior matches that for primitives.

Strictly, the spec does store non-callable objects (they just never get invoked); we clear those too, because keeping a value we could never call would only defer the failure to dispatch time. I've documented that deliberate narrowing in a code comment and added a regression test (should coerce a non-callable on<event> assignment to null) pinning the no-throw behavior.

On onabort (XMLHttpRequest.cpp:138): good catch, this one was a real defect -- Abort() only called m_request.Abort(), so the completion continuation reported the cancellation as a transport error and onabort was dead API. Abort() now records the intent and the continuation raises abort + loadend instead of error, per the DOM. Covered by a new test asserting that aborting an in-flight request fires abort and never error/load.

While in here I also hardened RaiseEvent along the lines of FileReader::Dispatch: it now snapshots the handler list before dispatching (a handler calling addEventListener/removeEventListener, or reassigning an on<event> property, could reallocate the vector or rehash the map out from under the in-flight dispatch -- a use-after-free) and clears pending exceptions between handlers so a throwing handler neither aborts the remaining dispatch nor escapes into the native completion continuation.

bghgary

[Reviewed by Copilot on behalf of @bghgary]

Two inline.

Addresses review feedback on BabylonJS#221. The on<event> properties lived in a parallel map dispatched ahead of the addEventListener list, which diverged from the DOM in two ways: - Dispatch ignored registration order. addEventListener("load", a) followed by xhr.onload = b called b then a; browsers call a then b. - xhr.onload = f; xhr.addEventListener("load", f) threw, where a browser registers both and calls f twice. Both kinds of listener now share one vector per event type, tagged with isEventHandler. The setter replaces the flagged entry in place so reassignment keeps its position, matching "If eventHandler's listener is not null, then return"; it appends when absent and erases when the assigned value is not callable. The duplicate check in addEventListener and the match in removeEventListener both skip the flagged entry, since those operate on addEventListener registrations only. Also narrow the failure test so a completed HTTP transaction dispatches 'load' regardless of status: const bool failed = result.has_error() || statusCode == 0; Per spec 'error' is for network-level failure; a 404 fires 'load' and callers branch on xhr.status. UrlStatusCode::None (0) is UrlLib's "no response obtained" sentinel -- it is only ever the initial value and the reset in ResetForOpen, because every path producing a response assigns an explicit code, including non-HTTP local file reads which set Ok. So the missing-local-file-on-UWP case that this condition was widened for still reports 'error'. Tests: both 404 tests now assert load fired and error did not, plus new coverage for cross-style dispatch order, position on reassignment, a function registered both ways being called twice, and removeEventListener not removing an on<event> handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88569c10-a7ff-4373-9a58-afa9c68b8c09

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (6)

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp:106

[LegacyTreatNonObjectAsNull] only converts non-object values to null; a non-callable object must fail callback-function conversion with a TypeError. This branch silently clears assignments such as xhr.onload = {}, which differs from the WebIDL contract. Handle primitive values separately and reject non-callable objects.

        if (!value.IsFunction())

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp:335

m_aborted is sticky and is never reset. Calling abort() before a request, or reusing an instance after an aborted request, therefore causes a later successful transfer to dispatch abort instead of load. Scope this flag and the underlying Abort() call to an active send, and reset the per-transfer state when a new send begins.

        m_aborted = true;

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp:432

Clearing the unified list now also clears every on<event> property. After completion xhr.onload reads back as null, and reusing the XHR loses all registered listeners; EventTarget registrations should persist until explicitly removed or the object is destroyed.

                m_listeners.clear();

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp:453

Snapshotting bare callback functions makes listener mutations during dispatch ineffective. If an earlier callback removes a later listener, the removed function remains in handlers and is still invoked; similarly, reassigning a pending onload invokes the old snapshot. Preserve stable listener records and check their current/removed state before each invocation.

        // Snapshot the handlers before dispatching. A handler may call addEventListener,
        // removeEventListener, or reassign an on<event> property while it runs, which would
        // otherwise reallocate the vector or rehash the map out from under this dispatch.
        // (Mirrors FileReader::Dispatch.)
        std::vector<Napi::Function> handlers{};

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp:472

This overload invokes the callback with an undefined receiver and no arguments. XHR listeners and handler properties must receive an event and run with this/currentTarget set to the XHR, so code using event.target or this.status breaks. Pass the wrapper object and an event value, as FileReader::Dispatch does in Polyfills/File/Source/FileReader.cpp:223-255.

            handler.Call({});

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp:158

The public Polyfills/XMLHttpRequest/Readme.md:5-8 still says onload-style properties are unsupported, omits load/abort, and says non-2xx responses fire error. Update that documentation alongside these accessors so users are not directed away from the newly supported API or given the old error semantics.

This issue also appears in the following locations of the same file:

line 335
line 432
line 449
line 472

                // DOM `on<event>` handler properties. Without these, `xhr.onreadystatechange = fn`
                // silently sets an ordinary expando property that is never invoked, so code written
                // against the standard XMLHttpRequest API waits forever for a callback that can
                // never fire.
                InstanceAccessor("onreadystatechange", &XMLHttpRequest::GetEventHandler<EventIndex::ReadyStateChange>, &XMLHttpRequest::SetEventHandler<EventIndex::ReadyStateChange>),

Addresses review feedback on BabylonJS#221. The on<event> properties lived in a parallel map dispatched ahead of the addEventListener list, which diverged from the DOM in two ways: - Dispatch ignored registration order. addEventListener("load", a) followed by xhr.onload = b called b then a; browsers call a then b. - xhr.onload = f; xhr.addEventListener("load", f) threw, where a browser registers both and calls f twice. Both kinds of listener now share one vector per event type, tagged with isEventHandler. The setter replaces the flagged entry in place so reassignment keeps its position, matching "If eventHandler's listener is not null, then return"; it appends when absent and erases when the assigned value is not callable. The duplicate check in addEventListener and the match in removeEventListener both skip the flagged entry, since those operate on addEventListener registrations only. Also narrow the failure test so a completed HTTP transaction dispatches 'load' regardless of status: const bool failed = result.has_error() || statusCode == 0; Per spec 'error' is for network-level failure; a 404 fires 'load' and callers branch on xhr.status. UrlStatusCode::None (0) is UrlLib's "no response obtained" sentinel -- it is only ever the initial value and the reset in ResetForOpen, because every path producing a response assigns an explicit code, including non-HTTP local file reads which set Ok. So the missing-local-file-on-UWP case that this condition was widened for still reports 'error'. Tests: both 404 tests now assert load fired and error did not, plus new coverage for cross-style dispatch order, position on reassignment, a function registered both ways being called twice, and removeEventListener not removing an on<event> handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88569c10-a7ff-4373-9a58-afa9c68b8c09

matthargett · 2026-09-16T22:29:39Z

Heads-up on a rooting hazard that this PR mirrors from FileReader::Dispatch ("Mirrors FileReader::Dispatch"): the snapshot is a std::vector<Napi::Function> of raw values held across the listener calls. On JavaScriptCore nothing roots a napi_value that lives only on the C++ heap (the backend's handle scopes are stubs and it scans just the C stack), so if a listener removes a later listener during dispatch, that later Napi::Function can be collected before the loop reaches it — the same class of bug as the CompressionStream output-chunk corruption fixed on #211, which reproduces deterministically under JSC_collectContinuously=1. Snapshotting the FunctionReferences (or a JS array) instead of the values avoids it. I was going to send a small PR for FileReader; since this PR adds a second copy of the pattern, either I fold both into one follow-up after this lands, or you take the FunctionReference snapshot here and I do FileReader only — your call.

matthargett · 2026-09-16T23:45:39Z

Follow-up: the FileReader side of this is now its own PR, #248 (snapshot Napi::FunctionReferences via Napi::Persistent instead of bare values, released when dispatch returns). If you mirror that shape in XMLHttpRequest::Dispatch here, both stay consistent; happy to rebase #248 on top of this if it lands first.

Addresses review feedback on BabylonJS#221. The on<event> properties lived in a parallel map dispatched ahead of the addEventListener list, which diverged from the DOM in two ways: - Dispatch ignored registration order. addEventListener("load", a) followed by xhr.onload = b called b then a; browsers call a then b. - xhr.onload = f; xhr.addEventListener("load", f) threw, where a browser registers both and calls f twice. Both kinds of listener now share one vector per event type, tagged with isEventHandler. The setter replaces the flagged entry in place so reassignment keeps its position, matching "If eventHandler's listener is not null, then return"; it appends when absent and erases when the assigned value is not callable. The duplicate check in addEventListener and the match in removeEventListener both skip the flagged entry, since those operate on addEventListener registrations only. Also narrow the failure test so a completed HTTP transaction dispatches 'load' regardless of status: const bool failed = result.has_error() || statusCode == 0; Per spec 'error' is for network-level failure; a 404 fires 'load' and callers branch on xhr.status. UrlStatusCode::None (0) is UrlLib's "no response obtained" sentinel -- it is only ever the initial value and the reset in ResetForOpen, because every path producing a response assigns an explicit code, including non-HTTP local file reads which set Ok. So the missing-local-file-on-UWP case that this condition was widened for still reports 'error'. Tests: both 404 tests now assert load fired and error did not, plus new coverage for cross-style dispatch order, position on reassignment, a function registered both ways being called twice, and removeEventListener not removing an on<event> handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88569c10-a7ff-4373-9a58-afa9c68b8c09

bghgary

[Reviewed by Copilot on behalf of @bghgary]

Concerns inline.

bghgary

[Reviewed by Copilot on behalf of @bghgary]

Two concerns remain.

Copilot

Copilot review overview

🟡 Changes recommended

Event-handler conversion, completed-request abort state, and exception-time propagation stopping remain incorrect.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 2

Open (3)

Reset completed XHR response state on abort · New
Reject non-callable object event handlers · New
Check immediate-propagation stop after listener exception · New

`XMLHttpRequest::RaiseEvent` only dispatches to handlers stored in `m_eventHandlerRefs`, which is populated exclusively by `addEventListener`. The class exposed no accessors for the DOM `on<event>` handler properties, so `xhr.onreadystatechange = fn` merely created an ordinary expando property on the JS wrapper that nothing ever read. The failure mode is silent and severe: the request runs to completion and `readyState`/`status` are updated correctly, but the callback never fires, so code written against the standard XMLHttpRequest API waits forever for an event that cannot arrive. There is no error and no diagnostic -- it simply hangs. Add `onreadystatechange`, `onload`, `onerror`, `onloadend` and `onabort` as instance accessors, stored in a separate map from the `addEventListener` handlers because they have assignment semantics (setting replaces the previous handler) rather than accumulating, and because they must be individually readable and clearable via `xhr.onload = null`. `RaiseEvent` now dispatches the `on<event>` handler in addition to any `addEventListener` handlers, matching the DOM, and `Send` releases the new strong references alongside the existing ones. Also raise the `load` event on success. It was previously never raised at all, so neither `onload` nor `addEventListener("load", ...)` could fire; only `loadend` and (on failure) `error` were dispatched. Success now dispatches `load` then `loadend`, and failure continues to dispatch `error` then `loadend`, per the spec. Adds five regression tests covering handler invocation on success and on HTTP 404, get/replace/clear semantics of the property, and co-existence with `addEventListener`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88569c10-a7ff-4373-9a58-afa9c68b8c09

Addresses review feedback on the `on<event>` handler properties: - `onabort` was exposed but no `abort` event was ever dispatched, so the handler could never fire. `Abort()` now records the caller's intent and the completion continuation reports the outcome as `abort` + `loadend` instead of `error`, matching the DOM. - `RaiseEvent` now snapshots the handler list before dispatching. A handler is free to call `addEventListener`/`removeEventListener` or reassign an `on<event>` property, either of which would reallocate the vector or rehash the map out from under an in-flight dispatch. It also clears pending exceptions between handlers so a throwing handler neither aborts the rest of the dispatch nor escapes into the native completion continuation. This mirrors `FileReader::Dispatch`. - Documented why a non-callable assignment clears the handler rather than throwing: `EventHandler` attributes are `[LegacyTreatNonObjectAsNull]` in WebIDL, so `xhr.onload = 0` yields `null` rather than a TypeError. Adds regression tests for the abort event and the non-callable coercion.

Addresses review feedback on BabylonJS#221. The on<event> properties lived in a parallel map dispatched ahead of the addEventListener list, which diverged from the DOM in two ways: - Dispatch ignored registration order. addEventListener("load", a) followed by xhr.onload = b called b then a; browsers call a then b. - xhr.onload = f; xhr.addEventListener("load", f) threw, where a browser registers both and calls f twice. Both kinds of listener now share one vector per event type, tagged with isEventHandler. The setter replaces the flagged entry in place so reassignment keeps its position, matching "If eventHandler's listener is not null, then return"; it appends when absent and erases when the assigned value is not callable. The duplicate check in addEventListener and the match in removeEventListener both skip the flagged entry, since those operate on addEventListener registrations only. Also narrow the failure test so a completed HTTP transaction dispatches 'load' regardless of status: const bool failed = result.has_error() || statusCode == 0; Per spec 'error' is for network-level failure; a 404 fires 'load' and callers branch on xhr.status. UrlStatusCode::None (0) is UrlLib's "no response obtained" sentinel -- it is only ever the initial value and the reset in ResetForOpen, because every path producing a response assigns an explicit code, including non-HTTP local file reads which set Ok. So the missing-local-file-on-UWP case that this condition was widened for still reports 'error'. Tests: both 404 tests now assert load fired and error did not, plus new coverage for cross-style dispatch order, position on reassignment, a function registered both ways being called twice, and removeEventListener not removing an on<event> handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88569c10-a7ff-4373-9a58-afa9c68b8c09

Re-adding an identical (type, callback) pair threw "Cannot add the same event handler twice". Per DOM the second add is a silent no-op, so the throw made valid browser code fail against the polyfill. The scan still skips `isEventHandler` entries, so `xhr.onload = f` followed by `xhr.addEventListener("load", f)` remains two independent registrations and still calls `f` twice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48268912-5d88-4e04-93ca-0c5cd35a03ad

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Guard abort and completion event sequences against replacement sends; construct Event/ProgressEvent instances with dispatch lifecycle and stop-immediate-propagation semantics. Cover reentrant callbacks and event prototypes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

bkaradzic-microsoft · 2026-09-28T16:57:49Z

Rebased onto latest main (62818ee, including the #257 test-layout split). Native/JS coverage now lives under Tests/UnitTests/Source/ (Tests.XMLHttpRequest.cpp + tests.xmlHttpRequest.ts); app:/// fixtures use Assets/ instead of Scripts/.

The describe() call was missing its closing `);` after the BabylonJS#257 test-file split, which broke the webpack/babel build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bghgary

[Reviewed by Copilot on behalf of @bghgary]

Concerns inline.

Prepare replacement requests before invalidating active sends and preserve responseType. Detach canceled transport state before synchronous callbacks and guard response access while the transport is active. Keep callbacks in a JavaScript WeakMap keyed by their XHR, with stable native listener records and short-lived persistent invocation references. Retain non-callable handler objects by identity without invoking them. Pin the Android response-read fix from BabylonJS/UrlLib#38. Add reuse, failed-open, object-identity, self-capturing GC, streaming-abort, and truncated HTTP response regressions, and update the polyfill documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The new loopback regression exposed NSURLSession completing a truncated 404 body without an NSError. Pick up explicit fixed-length identity-body validation from the companion UrlLib PR, retaining the original XHR regression expectations on all POSIX platforms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2b2272ba-79f8-41d0-aae1-f514a0f712ad

bkaradzic-microsoft · 2026-10-06T01:38:14Z

Fixed the four macOS CI failures in 972edff by updating the companion UrlLib pin to 85e8f026d8cb54fd6ed298b6c0be9bdadb67dcc8 (BabylonJS/UrlLib#38).

NSURLSession could report success for a truncated HTTP 404 body without an NSError. The Apple transport now rejects incomplete fixed-length identity-encoded bodies before publishing the response. The original XHR regression expectations are unchanged; dedicated UrlLib regressions also cover compressed, chunked, and bodyless responses to protect those cases.

Verification: all 24 jobs passed in CI run 37398377412, including all four previously failing macOS configurations and ThreadSanitizer. Companion UrlLib CI is also green.

Copilot

Copilot review overview

🔵 Needs a closer look

Cross-platform lifetime and cancellation changes, unresolved correctness issues, and the unmerged transport dependency warrant human review.

Review effort: Balanced
Findings: 2

Open (2)

Fix Event prototype stopImmediatePropagation state tracking · New
Avoid requiring NewTarget to inherit the XMLHttpRequest factory · New

Resolved since last review (3)

Reset completed XHR response state on abort
Check immediate-propagation stop after listener exception
Reject non-callable object event handlers

bkaradzic-microsoft · 2026-10-06T19:58:04Z

+                        if (this.cancelable) this.defaultPrevented = true;
+                    };
+                    Event.prototype.stopPropagation = function () { this.cancelBubble = true; };
+                    Event.prototype.stopImmediatePropagation = function () { this.cancelBubble = true; };


Fixed in c1a9433. The installed Event shim and XHR dispatch now share a weak per-event immediate-stop flag, so Event.prototype.stopImmediatePropagation.call(event) skips later listeners while stopPropagation() alone does not. Added a regression for both prototype calls; the QuickJS JavaScript suite passes.

bkaradzic-microsoft · 2026-10-06T19:58:04Z

    XMLHttpRequest::XMLHttpRequest(const Napi::CallbackInfo& info)
        : Napi::ObjectWrap<XMLHttpRequest>{info}
        , m_runtimeScheduler{JsRuntime::GetFromJavaScript(info.Env())}
+        , m_makeEvent{Napi::Persistent(info.NewTarget().As<Napi::Object>().Get(EVENT_FACTORY_NAME).As<Napi::Function>())}


Fixed in c1a9433. XHR gets its event factory from the native constructor registered during Initialize(), rather than from info.NewTarget(), so a forwarding constructor need not inherit the private static property. Added a Reflect.construct regression using a different new target; it passes on QuickJS and V8 CI is running.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Copilot AI lite review requested due to automatic review settings August 6, 2026 23:34

Copilot started reviewing on behalf of bkaradzic-microsoft August 6, 2026 23:46 View session

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp Outdated

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp

bkaradzic-microsoft requested a review from bghgary August 10, 2026 13:37

bghgary requested changes Aug 10, 2026

View reviewed changes

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp Outdated

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h Outdated

bkaradzic-microsoft requested a review from bghgary August 11, 2026 01:02

bkaradzic-microsoft requested a balanced review from Copilot August 13, 2026 17:51

Copilot started reviewing on behalf of bkaradzic-microsoft August 13, 2026 17:51 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

bkaradzic-microsoft force-pushed the fix-xhr-on-event-handlers branch from bdebbfc to 5280ae2 Compare September 14, 2026 19:45

This was referenced Sep 16, 2026

File: keep the FileReader listener snapshot rooted while dispatching #248

Open

[fork twin] File: keep the FileReader listener snapshot rooted while dispatching rebeckerspecialties/JsRuntimeHost#32

Open

bkaradzic-microsoft force-pushed the fix-xhr-on-event-handlers branch from 5280ae2 to c07d7cf Compare September 22, 2026 20:06

bghgary requested changes Sep 22, 2026

View reviewed changes

bghgary reviewed Sep 22, 2026

View reviewed changes

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp Outdated

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp Outdated

bkaradzic-microsoft requested review from bghgary and a balanced review from Copilot September 22, 2026 22:57

Copilot started reviewing on behalf of bkaradzic-microsoft September 23, 2026 20:49 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp Outdated

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp Outdated

Comment thread Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp

bkaradzic and others added 5 commits September 28, 2026 09:54

Enforce WebIDL event-handler conversion

7f0f615

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Branimir Karadzic and others added 7 commits September 28, 2026 09:55

Complete XMLHttpRequest event semantics

f23fae8

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Keep XHR event dispatch compatible with JSI

39ef5f7

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Snapshot XHR status on the JavaScript thread

0a7cec6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Correct XHR event handler conversion and completed abort state

d9dcd64

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Preserve JavaScript errors thrown by JSI instance setters

b46ee7d

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Finish XHR request before testing throwing listener cleanup

5d9ccae

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

bkaradzic-microsoft force-pushed the fix-xhr-on-event-handlers branch from ae9a4a9 to 5d9ccae Compare September 28, 2026 16:57

Fix XMLHttpRequest test suite closing delimiter

ad60f55

The describe() call was missing its closing `);` after the BabylonJS#257 test-file split, which broke the webpack/babel build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bkaradzic-microsoft mentioned this pull request Sep 29, 2026

XMLHttpRequest: report URL-open failures asynchronously from send #260

Open

bghgary requested changes Oct 5, 2026

View reviewed changes

bkaradzic-microsoft mentioned this pull request Oct 5, 2026

Report Android and Apple response-body failures as transport errors BabylonJS/UrlLib#38

Open

bkaradzic and others added 2 commits October 5, 2026 16:44

bkaradzic-microsoft requested review from bghgary and a balanced review from Copilot October 6, 2026 14:46

Copilot started reviewing on behalf of bkaradzic-microsoft October 6, 2026 14:53 View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

Fix XHR event propagation and constructor forwarding

c1a9433

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305

Conversation

bkaradzic-microsoft commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Regression coverage

Compatibility

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

bkaradzic-microsoft commented Aug 7, 2026

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

matthargett commented Sep 16, 2026

Uh oh!

matthargett commented Sep 16, 2026

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkaradzic-microsoft commented Sep 28, 2026

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkaradzic-microsoft commented Oct 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

bkaradzic-microsoft Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

bkaradzic-microsoft Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

bkaradzic-microsoft commented Aug 6, 2026 •

edited

Loading