Status

()

RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: csthomas, Assigned: csthomas)

Tracking

Trunk
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

JavaScript strict warning: chrome://browser/content/browser.js, line 6646: trailing comma is not legal in ECMA-262 object initializers
JavaScript strict warning: chrome://browser/content/places/toolbar.xml, line 623: function TBV_DO_onDragStart does not always return a value
Created attachment 232475 [details] [diff] [review]
patch [checked in]

return a value consistently, and remove spurious commas
Attachment #232475 - Flags: review?(mconnor)
Status: NEW → ASSIGNED
Component: General → Places
QA Contact: general → places
Created attachment 232476 [details] [diff] [review]
fix another one

When dropping a tab after the last tab, the old code resulted in an out-of-bounds array access.  Using .item() avoids the warning and returns null more cleanly.
Attachment #232476 - Flags: review?(mconnor)
Component: Places → General
QA Contact: places → general
(In reply to comment #2)
> Created an attachment (id=232476) [edit]
> fix another one
> 
> When dropping a tab after the last tab, the old code resulted in an
> out-of-bounds array access.  Using .item() avoids the warning and returns null
> more cleanly.

This one affects the 1.8 branch, too. Might want to request approval1.8.1 on it (couldn't hurt!). :)

Comment 4

12 years ago
I'm not entirely certain [] and .item() aren't synonymous, and if I'm right this looks like a bug.

Comment 5

12 years ago
(In reply to comment #4)
>I'm not entirely certain [] and .item() aren't synonymous, and if I'm right
>this looks like a bug.
FYI foo[a] is roughly equivalent to
0 <= a && a < foo.length ? foo.item(a) : (report_strict_warning(), undefined);
Whiteboard: [cst: r?]
Comment on attachment 232475 [details] [diff] [review]
patch [checked in]

Trying a different reviewer.
Attachment #232475 - Flags: review?(mconnor) → review?(mano)
Attachment #232476 - Flags: review?(mconnor) → review?(mano)
Comment on attachment 232476 [details] [diff] [review]
fix another one

If there's an out-of-bounds issue here, let's fix that instead.
Attachment #232476 - Flags: review?(mano) → review-
Comment on attachment 232475 [details] [diff] [review]
patch [checked in]

r=mano
Attachment #232475 - Flags: review?(mano) → review+
Attachment #232475 - Attachment description: patch → patch [checked in]
Created attachment 243144 [details] [diff] [review]
fix another one, with comment explaining why this is correct [checked in]

Added comment explaining why you need to use .item so nobody "cleans up" this code in the future.
Attachment #232476 - Attachment is obsolete: true
Attachment #243144 - Flags: review?(mano)
Comment on attachment 243144 [details] [diff] [review]
fix another one, with comment explaining why this is correct [checked in]

ok, r=mano.
Attachment #243144 - Flags: review?(mano) → review+
Attachment #243144 - Attachment description: fix another one, with comment explaining why this is correct → fix another one, with comment explaining why this is correct [checked in]
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [cst: r?]

Updated

12 years ago
Duplicate of this bug: 333621
You need to log in before you can comment on or make changes to this bug.