153660 - |f.arguments| causes "deprecated arguments usage" warning

Status: VERIFIED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[timeless]

Warnung: deprecated arguments usage

Quelldatei: http://www.electran.org/

Zeile: 263
Warnung: reference to undefined property makeArray0.arguments

Quelldatei: http://www.electran.org/

Zeile: 262
Warnung: deprecated arguments usage

Quelldatei: http://www.electran.org/

Zeile: 94
Warnung: reference to undefined property changeImages.arguments

Quelldatei: http://www.electran.org/

Zeile: 95

I'd like to eventually make this a Tech Evang bug, but i'm having trouble 
interpretting the warning and was wondering what suggestion to provide to the 
site author.

[sorry for the German, i'm testing live local/skin switching and german's 
what's available]

Comment 1

[Eric Hodel]

There is some seriously funky javascript on this page:

function makeArray0() {
  for (i = 0; i<makeArray0.arguments.length; i++)
    this[i] = makeArray0.arguments[i];
}

...

var months = new makeArray0('January','February','March',
    'April','May','June','July','August','September',
    'October','November','December');

...

document.write("Last updated on " + months[dateY2K.getMonth()] + " " +
...

Should be just:

var months = ['January', ...];

in fact, Date.toLocaleDateString() would probably be the best piece of code to
use here.  It seems really weird to do Y2K correction...

Also:

sitemapon = new Image();
sitemapon.src = "images/topnav_sitemap_r.gif";

sitemapoff = new Image();
sitemapoff.src = "images/topnav_sitemap.gif";

function changeImages() {
  if (document.images) {
    for (var i=0; i<changeImages.arguments.length; i+=2) {
      document[changeImages.arguments[i]].src = eval(changeImages.arguments[i+1]
+ ".src");
    }
  }
}

...

<a HREF="sitemap.html" onMouseOver="changeImages('sitemap', 'sitemapon')"
onMouseOut="changeImages('sitemap', 'sitemapoff')"><img
SRC="images/topnav_sitemap.gif" name="sitemap" width="70" height="20" border="0"
alt="Site Map"></a>

Could be:

var sitemaponsrc = "images/topnav_sitemap_r.gif";
var sitemapoffsrc = "images/topnav_sitemap.gif";

function changeImageTo(imgsrc) {
  document.getElementById(sitemap).src = imgsrc;
}

<a href="sitemap.html" onMouseOver="changeImageTo(sitemaponsrc);" 
onMouseOut="changeImageTo(sitemapoffsrc);"><img id="sitemap" ...></a>

Which will be much faster, as it avoids an eval()

Comment 2

[Phil Schwartau]

timeless, could you please give steps to reproduce this?
I get no errors loading this site, and all I'm doing is clicking
on things and getting nowhere.

This doesn't require a login, does it? If so, could you please
provide one.

Finally, I am unable to match the line numbers in the errors 
reported above (e.g. on line 263, line 262) to the file I'm seeing
at http://www.electran.org/.

The file I'm seeing only has 187 lines -

Comment 3

[timeless]

according to nc4/notepad (nc4 {view-source:http://www.electran.org/, 
select-all, copy}, notepad{paste, ctrl-end, ctrl-g})
http://www.electran.org/ has 305 lines.

i filed the bug because when mozilla just loaded the page (well actually, my 
random home page turned out to be that page), it loaded and flooded my js 
console. i don't think i interacted with the page at all.

line 263 <!--
function makeArray0() {
     for (i = 0; i<makeArray0.arguments.length; i++)
          this[i] = makeArray0.arguments[i];
}

the bottom of the generated page shows 'Last updated on December 31, 2069.' in 
nc4 :-). I can't believe that these people would actually do server side 
sniffing, so my guess is your browser messed up its line numbers.

are you sure you have strict warnings enabled ;-?

Comment 4

[Phil Schwartau]

We can reduce this to the following testcase, which I will attach below.

<script>
  function f() { return f.arguments.length }
  function g() { return arguments.length   }
  f();
  g();
</script>

Comment 5

[Phil Schwartau]

Attachment: HTML testcase

Comment 6

[Phil Schwartau]

STEPS TO REPRODUCE:

1. Turn JavaScript string warnings on in Mozilla
2. Load the testcase and look in the JS Console
3. For the time being, let's ignore bug 104549,
  "Too many duplicate warnings in JS Console...with strict warnings enabled"
4. We see two distinct warnings: 

   Warning: reference to undefined property f.arguments
   Source File: http://bugzilla.mozilla.org/attachment.cgi?id=88890&action=view
   Line: 3

   Warning: deprecated arguments usage
   Source File:http://bugzilla.mozilla.org/attachment.cgi?id=88890&action=view
   Line: 3

5. Note these are all coming from the function f() above; none from g().


Note the only difference between f() and g() is:

           function f() { return f.arguments.length }
           function g() { return arguments.length   }

So at one time, apparently it was considered good form to use the
function name when referring to the |arguments| object; and now it isn't.

I did find the following comment by doing some archaeology in the js testsuite.
See mozilla/js/tests/js1_4/Functions/function-001.js:

Author:       [e-mail]
Date:         11 August 1998
File Name:    function-001.js
Description:  http://scopus.mcom.com/bugsplat/show_bug.cgi?id=324455
 

"Earlier versions of JavaScript supported access to the arguments property
of the function object. This property held the arguments to the function:

    function f() 
    {
      return f.arguments[0];    // deprecated
    }
    var x = f(3);    // x will be 3
 
This feature is not a part of the final ECMA standard. Instead, scripts
should simply use just "arguments":
 
    function f() {
      return arguments[0];    // okay
    }
 
    var x = f(3);    // x will be 3
 
Again, this feature was motivated by performance concerns. Access to the 
arguments property is not threadsafe, which is of particular concern in
server environments. Also, the compiler can generate better code for functions 
because it can tell when the arguments are being accessed only by name
and avoid setting up the arguments object."

Comment 7

[Phil Schwartau]

This pretty much explains why the warnings are being produced.

cc'ing rogerl, waldemar, shaver, and Brendan in case they would
like to comment more authoritatively on why the arguments object
should be accessed via |arguments| and not |f.arguments|.

Note one can see that |'arguments' in f| still evaluates to true;
otherwise the scripts above would be producing ERRORS on |f.arguments|
instead of strict warnings. In that light, our warning above,

      Warning: reference to undefined property f.arguments

seems like a bit of a lie!

Comment 8

[Phil Schwartau]

Correction to my Comment #7: referencing a property that has not been
defined does not cause an error, as I stated above. However, the |in| 
operator would return 'false', not 'true', which is what I was getting at:

js> var obj = {};
js> obj.a
2: strict warning: reference to undefined property obj.a
js> obj.a === undefined
3: strict warning: reference to undefined property obj.a
true
js> 'a' in obj
false

Comment 9

[Phil Schwartau]

From email from timeless, I think that it's really bug 104549 that was
his main concern here (see Comment #6). Therefore I'm going cc timeless
on that bug, raise its severity, and resolve/verify this bug as WORKSFORME.

If any of the developers can comment on why |arguments| is better than
|f.arguments|, could Christine's summary in Comment #6 be improved?

Comment 10

[Phil Schwartau]

Marking Verified Worksforme -