Commit c85c5b8
committed
Fix FragmentInstance listener leak: normalize boolean vs object capture options per DOM spec (#36047)
## Summary
`FragmentInstance.addEventListener` and `removeEventListener` fail to
cross-match listeners when the `capture` option is passed as a
**boolean** in one call and an **options object** in the other. This
violates the [DOM Living
Standard](https://dom.spec.whatwg.org/#dom-eventtarget-removeeventlistener),
which states that `addEventListener(type, fn, true)` and
`addEventListener(type, fn, {capture: true})` are identical.
### Root Cause
In `ReactFiberConfigDOM.js`, the `normalizeListenerOptions` function
generates a listener key string for deduplication. The boolean branch
generates a **different format** than the object branch:
```js
// Boolean branch (old) — produces "c=1"
return `c=${opts ? '1' : '0'}`;
// Object branch — produces "c=1&o=0&p=0"
return `c=${opts.capture ? '1' : '0'}&o=${opts.once ? '1' : '0'}&p=${opts.passive ? '1' : '0'}`;
```
Because the keys differ, `indexOfEventListener` cannot match them — so
`removeEventListener('click', fn, {capture: true})` silently fails to
remove a listener registered with `addEventListener('click', fn, true)`,
and vice versa. This causes a **memory leak and event listener
accumulation** on all Fragment child DOM nodes.
### Fix
Normalize the boolean branch to produce the same full key format:
```js
// Boolean branch (fixed) — now produces "c=1&o=0&p=0" (matches object branch)
return `c=${opts ? '1' : '0'}&o=0&p=0`;
```
This makes both forms produce an identical key, matching the DOM spec
behavior.
### When Was This Introduced
This bug has been present since `FragmentInstance` event listener
tracking was first added. It became reachable in production as of
[#36026](#36026) which enabled
`enableFragmentRefs` + `enableFragmentRefsInstanceHandles` across all
builds (merged 3 days ago).
### Tests
Added two new regression tests to `ReactDOMFragmentRefs-test.js`:
1. `removes a capture listener registered with boolean when removed with
options object`
2. `removes a capture listener registered with options object when
removed with boolean`
Both tests were failing before this fix and pass after.
## How did you test this change?
Added two new automated tests covering both cross-form removal
directions. Existing tests continue to pass.
## Changelog
### React DOM
- **Fixed** `FragmentInstance.removeEventListener()` not removing
capture-phase listeners when the `capture` option form (boolean vs
options object) differs between `add` and `remove` calls.
DiffTrain build for [142cfde](142cfde)1 parent 02ee372 commit c85c5b8
34 files changed
Lines changed: 96 additions & 146 deletions
File tree
- compiled/facebook-www
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1482 | 1482 | | |
1483 | 1483 | | |
1484 | 1484 | | |
1485 | | - | |
| 1485 | + | |
1486 | 1486 | | |
1487 | 1487 | | |
1488 | 1488 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1482 | 1482 | | |
1483 | 1483 | | |
1484 | 1484 | | |
1485 | | - | |
| 1485 | + | |
1486 | 1486 | | |
1487 | 1487 | | |
1488 | 1488 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
610 | 610 | | |
611 | 611 | | |
612 | 612 | | |
613 | | - | |
| 613 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
610 | 610 | | |
611 | 611 | | |
612 | 612 | | |
613 | | - | |
| 613 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
614 | 614 | | |
615 | 615 | | |
616 | 616 | | |
617 | | - | |
| 617 | + | |
618 | 618 | | |
619 | 619 | | |
620 | 620 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
614 | 614 | | |
615 | 615 | | |
616 | 616 | | |
617 | | - | |
| 617 | + | |
618 | 618 | | |
619 | 619 | | |
620 | 620 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20434 | 20434 | | |
20435 | 20435 | | |
20436 | 20436 | | |
20437 | | - | |
| 20437 | + | |
20438 | 20438 | | |
20439 | 20439 | | |
20440 | | - | |
| 20440 | + | |
20441 | 20441 | | |
20442 | 20442 | | |
20443 | 20443 | | |
| |||
20472 | 20472 | | |
20473 | 20473 | | |
20474 | 20474 | | |
20475 | | - | |
| 20475 | + | |
20476 | 20476 | | |
20477 | 20477 | | |
20478 | 20478 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20205 | 20205 | | |
20206 | 20206 | | |
20207 | 20207 | | |
20208 | | - | |
| 20208 | + | |
20209 | 20209 | | |
20210 | 20210 | | |
20211 | | - | |
| 20211 | + | |
20212 | 20212 | | |
20213 | 20213 | | |
20214 | 20214 | | |
| |||
20243 | 20243 | | |
20244 | 20244 | | |
20245 | 20245 | | |
20246 | | - | |
| 20246 | + | |
20247 | 20247 | | |
20248 | 20248 | | |
20249 | 20249 | | |
| |||
0 commit comments