347683 - strict warnings

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

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

Comment 1

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch [checked in]

return a value consistently, and remove spurious commas

Comment 2

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: 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.

Comment 3

[Reed Loden [:reed]]

(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

[Jeff Walden [:Waldo]]

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

Comment 5

[[e-mail]]

(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);

Comment 6

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 232475 [details] [diff] [review]
patch [checked in]

Trying a different reviewer.

Comment 7

[Asaf Romano (gone)]

Comment on attachment 232476 [details] [diff] [review]
fix another one

If there's an out-of-bounds issue here, let's fix that instead.

Comment 8

[Asaf Romano (gone)]

Comment on attachment 232475 [details] [diff] [review]
patch [checked in]

r=mano

Comment 9

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: 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.

Comment 10

[Asaf Romano (gone)]

Comment on attachment 243144 [details] [diff] [review]
fix another one, with comment explaining why this is correct [checked in]

ok, r=mano.