Regression: Array index has different results

VERIFIED FIXED in mozilla1.9.1

Status

()

P1
critical
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: sroussey, Assigned: brendan)

Tracking

(4 keywords)

unspecified
mozilla1.9.1
fixed1.9.0.2, regression, testcase, verified1.9.1
Points:
---
Bug Flags:
blocking1.9.1 +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1
Build Identifier: 

Indexing an array by number, if that number is then quoted, results in undefined, where all previous versions of Firefox (as well as IE, Safari, etc) implicitly convert the number string into a number for the index and return that part of the array.

Reproducible: Always

Steps to Reproduce:
var s=[1,2,3]
console.log(s['1')
Actual Results:  
undefined

Expected Results:  
2

Updated

10 years ago
Duplicate of this bug: 451935

Updated

10 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

10 years ago
This should block 1.9.1.

/be
Severity: major → critical
OS: Windows Vista → All
Hardware: PC → All

Updated

10 years ago
Flags: blocking1.9.1+
(Assignee)

Comment 3

10 years ago
Created attachment 335403 [details] [diff] [review]
fix

if (is_special_case) {
  special_case();
} else {
  normal_case();
}

when you want to split special_case() into a more special version of itself and a variation on normal_case() becomes

do {
  if (is_special_case) {
    if (extra_special) {
      special_case();
      break;
    }
    setup_variation_on_normal_case();
  }
  normal_case();
} while (0);

Maximum code sharing without goto.

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #335403 - Flags: review?(shaver)
(Assignee)

Comment 4

10 years ago
Want this in the next possible patch release too.

/be
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1
Comment on attachment 335403 [details] [diff] [review]
fix

r=shaver; need test coverage here, for sure.
Attachment #335403 - Flags: review?(shaver) → review+
(Assignee)

Updated

10 years ago
Duplicate of this bug: 450273
(Assignee)

Comment 7

10 years ago
Fixed:

http://hg.mozilla.org/mozilla-central/index.cgi/rev/b11ad5053d28

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

10 years ago
Comment on attachment 335403 [details] [diff] [review]
fix

Can this make it for 1.9.0.2?

/be
Attachment #335403 - Flags: approval1.9.0.2?
(Assignee)

Comment 9

10 years ago
bc: you should holler harder when we break stuff ;-).

Sam: this is covered by the js testsuite -- we just didn't act on the bug bc filed, due to heads-down TM hacking. This is a pretty bad regression, and the fix should go into the next Firefox 3.0.x.

/be

Comment 10

10 years ago
actually, js1_5/Array/regress-107138.js didn't show a failure on 1.9.0 for me. :-(

I figured you saw my bug mail about the regressions on MC/TM. But I'll speak up more.
Flags: in-testsuite?
Flags: in-litmus-
Comment on attachment 335403 [details] [diff] [review]
fix

Approved for 1.9.0.2. Please land in CVS ASAP (code freeze is tonight at midnight PDT). a=ss

Note: I'm approving this knowing that any new tests that need to be created for it will get created shortly by Bob.
Attachment #335403 - Flags: approval1.9.0.2? → approval1.9.0.2+

Comment 12

10 years ago
Lands cleanly with no more than a name-change, so that's what I did:

Checking in jsparse.c;
/cvsroot/mozilla/js/src/jsparse.c,v  <--  jsparse.c
new revision: 3.349; previous revision: 3.348
done

Updated

10 years ago
Keywords: fixed1.9.0.2

Updated

10 years ago
Keywords: regression

Updated

10 years ago
Depends on: 452369

Comment 13

10 years ago
Steve, this was with Firebug correct? I can't reproduce this on mac or windows with Firefox 3.0.1 and Firebug 1.2.

Does anyone have a shell test that illustrates the bug on 1.9.0?
(Reporter)

Comment 14

10 years ago
Wasn't in 3.0.1 or I think a lot of people would have complained. ;) Was in Minefield when i discovered it (3.1a2pre of the bug report date).

Comment 15

10 years ago
That wasn't clear from the build info in the original report. The regression on 1.9.1 is covered by the duped bug's test. Brendan, what did you fix on 1.9.0?

Updated

10 years ago
Keywords: testcase

Comment 16

10 years ago
/cvsroot/mozilla/js/tests/js1_5/Array/regress-451906.js,v  <--  regress-451906.js
initial revision: 1.1

http://hg.mozilla.org/mozilla-central/rev/f0e9fd501e63
Flags: in-testsuite? → in-testsuite+
Keywords: fixed1.9.1

Comment 17

10 years ago
v 1.9.0, 1.9.1 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.