The default bug view has changed. See this FAQ.

Patch for bug 310351 was mildly bogus

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: mrbkap, Assigned: mrbkap)

Tracking

({fixed1.8, regression})

Trunk
x86
Linux
fixed1.8, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
My method of getting the length was incorrect for _generic_ arrays, where we
can't make any assumptions about our data. Instead, we should go through
javascript (and then back through XPConnect, ick.
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

12 years ago
Created attachment 198474 [details] [diff] [review]
As described
Attachment #198474 - Flags: superreview?(brendan)
Attachment #198474 - Flags: review?(jst)
Comment on attachment 198474 [details] [diff] [review]
As described

This is ok, but you might change the JSVAL_IS_INT fretting to cope with
unsigned 32 bit values that do not fit in the nonnegative half of the signed 31
bit domain of int jsvals.

But that's for later, on the trunk.  For this patch, just change the XXX
comment to say how this can really happen (sparse array delegating to the DOM
prototype).

/be
Attachment #198474 - Flags: superreview?(brendan) → superreview+
(Assignee)

Comment 3

12 years ago
Comment on attachment 198474 [details] [diff] [review]
As described

Prematurely requesting approval, this patch (re-)fixes maps.google.com
Attachment #198474 - Flags: approval1.8b5?
(Assignee)

Updated

12 years ago
Blocks: 311094

Updated

12 years ago
Blocks: 311107
(Assignee)

Updated

12 years ago
Blocks: 311104

Comment 4

12 years ago
I e-mailed jst directly this morning asking him to review this ASAP.
Blocks: 311067

Comment 5

12 years ago
Comment on attachment 198474 [details] [diff] [review]
As described

I'm going to approve this for the branch. jst, doesn't have internet
connectivity at the moment and we'd rather gets some bits up that we can all
start testing instead of waiting. 

Leaving the review request alone, so we can address any review comments from
jst later.
Attachment #198474 - Flags: approval1.8b5? → approval1.8b5+
(Assignee)

Comment 6

12 years ago
Checked in on trunk and MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Comment on attachment 198474 [details] [diff] [review]
As described

r=jst
Attachment #198474 - Flags: review?(jst) → review+
should Bug 311119 be fixed py this patch too ?
(Assignee)

Updated

12 years ago
Blocks: 311119
Hmm.. This had noticeable (0.5%?) Tp impact on btek.  Anything we can do about that?
(In reply to comment #9)
> Hmm.. This had noticeable (0.5%?) Tp impact on btek.  Anything we can do about
> that?

Sure -- at the cost of a QI test, restore the lost fast path that resulted in
this bug, and do the full get-property on 'length' if the QI to nsIDOMNodeList
fails.

/be

(Assignee)

Updated

12 years ago
Depends on: 311151
(Assignee)

Comment 11

12 years ago
bug 311151 filed to track the Tp regression.
Blocks: 311171
Blake, Bug 311204 has the same regressionwindow as all others here.
Could it have been caused by bug 310351 but not fixed by this bug ?
You need to log in before you can comment on or make changes to this bug.