Closed Bug 1767541 Opened 2 years ago Closed 2 years ago

Array includes breaks on sparse arrays created with the Array constructor

Categories

(Core :: JavaScript Engine, defect)

Firefox 99
defect

Tracking

()

RESOLVED FIXED
102 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox100 --- wontfix
firefox101 --- wontfix
firefox102 --- fixed

People

(Reporter: ljharb, Assigned: anba)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Steps to reproduce:

[,].includes() correctly returns true, because a sparse array of length 1 does include "undefined".

Array(1).includes() incorrectly returns false, because a sparse array of length 1 is supposed to include "undefined".

Additionally, [,] and Array(1) should never have differing behavior, since they're identical operations in the spec. Note this happens in both 99 and 100.

Component: Untriaged → JavaScript: Standard Library
Product: Firefox → Core
Component: JavaScript: Standard Library → JavaScript Engine
Assignee: nobody → andrebargull
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

We don't have to call StrictlyEqual for holes in the SearchKind::Includes
code path, because holes are treated as "undefined" and "undefined" values are
already handled in the CanUseBitwiseCompareForStrictlyEqual path.

Depends on D145407

Regressed by: 1755489

Set release status flags based on info from the regressing bug 1755489

Has Regression Range: --- → yes
Pushed by andre.bargull@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/11f5801a2506
Part 1: Correctly handle trailing holes in Array.prototype.includes. r=jandem
https://hg.mozilla.org/integration/autoland/rev/96e265267978
Part 2: Remove unreachable code in SearchElementDense generic path. r=jandem
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
Flags: in-testsuite+

The patch landed in nightly and beta is affected.
:anba, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(andrebargull)

I don't think we need to uplift these patches, because comment #0 doesn't indicate there's any website which is currently broken due to this bug.

Flags: needinfo?(andrebargull)

I'm sorry I missed this 2 months ago; this is forcing a workaround in some very large npm packages which have many tens of millions of weekly downloads.

It's violating the spec; this should be fixed ASAP.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

It's fixed in the current (version 102) release.

Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED

Awesome, thank you! I missed that.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: