102886 - javascript strict warnings in linkToolbarOverlay.js

Status: VERIFIED DUPLICATE of bug 103097

(SeaMonkey :: General, defect)

Description

[Henrik Gemal]

when using the javascript debugger (I think)...

Warning: assignment to undeclared variable LinkToolbarUI
Source File: chrome://navigator/content/linkToolbarOverlay.js
Line: 48

Comment 1

[Henrik Gemal]

caused by bug:
http://bugzilla.mozilla.org/show_bug.cgi?id=87428

Comment 2

[Christopher Hoess (gone)]

qa -> claudius.  Maybe caused by empty function declaration (I forget what the OO 
term for this is) for linkToolbarUI?

Comment 3

[Christopher Hoess (gone)]

WFM after applying my patch for bug 102985.

Comment 4

[Gervase Markham [:gerv]]

*** This bug has been marked as a duplicate of 102985 ***

Comment 5

[Christopher Hoess (gone)]

Whoops, wrong bug.

Comment 6

[Christopher Hoess (gone)]

*** This bug has been marked as a duplicate of 102895 ***

Comment 7

[Henrik Gemal]

this is not a dupe....
At least the warnings has NOT been fixed.

I'm still seeing:
Warning: assignment to undeclared variable hideMiscellaneousSeparator
Source File: chrome://navigator/content/linkToolbarItem.js
Line: 272
Warning: assignment to undeclared variable LinkToolbarUI
Source File: chrome://navigator/content/linkToolbarOverlay.js
Line: 50
Warning: assignment to undeclared variable showMiscellaneousSeparator
Source File: chrome://navigator/content/linkToolbarItem.js
Line: 266

Comment 8

[Stuart Ballard]

I've seen these too. Could someone with JS knowledge comment on what needs to be
done (in general terms) to get rid "undefined variable" warnings? I thought that
in JS you could create a variable on the fly by assigning to it with no
problems...

Comment 9

[Henrik Gemal]

yes you can but you cant just to:

<script>
x = 1;
</script>

You gotta have:
<script>
var x=1;
</script>

Comment 10

[Christopher Hoess (gone)]

Tim, do you think you could take a look at this? I don't understand these OO
conventions all that well, although I can fix the 2 errors in linkToolbarItem.js.

Comment 11

[Tim 'Tool-Man' Taylor]

Yes I can, but I won't have time until this weekend.

Comment 12

[basic]

what's the deal with these?

LinkToolbarUI = function()

showMiscellaneousSeparator = function()

hideMiscellaneousSeparator = function()

why not these?

function LinkToolBarUI()

function showMiscellaneousSeparator()

function hideMiscellaneousSeparator()

if there are no reasons, I'll attach a patch.

Comment 13

[basic]

Attachment: patch

Comment 14

[Tim 'Tool-Man' Taylor]

> if there are no reasons, I'll attach a patch.

There's no reason that I can tell.  Someone probably switched all function
definitions to the "foo = function()" style believing they were making things
consistent.  The "foo = function()" style only applies to OO programming and
prototype inheritance in JavaScript.  Ultimately, the right solution is to
switch to the "Foo.prototype = { ... }" style of defining classes.  The current
style is a holdover from when I was learning OO programming in JavaScript. 
Either way, the functions you pointed out aren't methods, so they should be
defined using the latter syntax.

Your patch gets r=Tool-Man, assuming I can give it.  I was lazy and didn't test.
   I'm assuming it works for you?

Comment 15

[Stuart Ballard]

my patch for bug 103097 also fixes these warnings; I'd personally rather see
that go in than this one because it also fixes the perf issues. It's got review
but it's waiting for sr and possibly approval if it's to make it for 0.9.7.
Unfortunately I don't have the time to chase up sr at this point, but I'd love
for someone else to.

However, these changes look good to me too (although I have no time to test
either). If you can find someone else to test the patch and make sure that it
works, r=[e-mail] if you want to check this one in instead of the
103097 patch. If you do check this one in, I'll re-diff that one.

Comment 16

[Alex Vincent [:WeirdAl]]

Heh, basic's patch also would fix bug 110438 (which I just spent a minute 
writing a patch for using Patch Maker)

Comment 17

[basic]

fixed in patch for bug 103097

*** This bug has been marked as a duplicate of 103097 ***

Comment 18

[Henrik Gemal]

Verified dupe