724441 - Avoid using arguments.callee() and just give every function (expression) a name, in SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

https://developer.mozilla.org/en/JavaScript/Reference/Functions_and_function_scope/arguments/callee#Description
{
Note: You should avoid using arguments.callee() and just give every function (expression) a name.

Warning: The 5th edition of ECMAScript forbids use of arguments.callee() in strict mode.
}

***

"Found 101 matching lines in 38 files"

All in tests, except
{
/suite/common/src/nsSessionStore.js
    * line 408 -- aEvent.currentTarget.removeEventListener("load", arguments.callee, false);

/suite/common/aboutSessionRestore.js
    * line 135 -- newWindow.removeEventListener("load", arguments.callee, true);

/suite/common/utilityOverlay.js
    * line 345 -- toolbox.removeEventListener("beforecustomization", arguments.callee, false);
    * line 1533 -- browserWin.removeEventListener("pageshow", arguments.callee, true);
}

***

Bug 724331 comment 13
{
[e-mail]      2012-02-05 14:30:19 PST

No rush, we just need to change them as and when we patch those uses.
}

That seems to be a good G.F.B. to me...
It can be done in chunks, in blocking bugs, as need be.

NB: It may be worth fixing Firefox code first, when SeaMonkey copies it.

Comment 1

[Edmund Wong (:ewong)]

Attachment: Replaced arguments.callee() with a name. (v1)

this patch also includes changes to the MPL headers.

Comment 2

[Philip Chee]

> this patch also includes changes to the MPL headers.
Don't do that Gerv is about to land a tree wide change to the MPL headers.

Comment 3

[[e-mail]]

Comment on attachment 628341 [details] [diff] [review]
Replaced arguments.callee() with a name. (v1)

>diff --git a/suite/common/aboutSessionRestore.js b/suite/common/aboutSessionRestore.js
[Don't need to bother with licence changes, Gerv will sort things out.]

>-    tab2.linkedBrowser.addEventListener("load", function(aEvent) {
>-      this.removeEventListener("load", arguments.callee, true);
>+    tab2.linkedBrowser.addEventListener("load", function anon1(aEvent) {
>+      this.removeEventListener("load", anon1, true);
Sorry, but I don't appreciate your choice of function names. Try to make up a meaningful, but not too long, name that would be useful in a stack trace. For instance, "tab2Loaded" could work well here. (Note that this is actually my third attempt at a function name, so feel free to improve on it.)

Comment 4

[Edmund Wong (:ewong)]

Attachment: Replaced arguments.callee() with a name. (v2)

Comment 5

[[e-mail]]

Comment on attachment 628617 [details] [diff] [review]
Replaced arguments.callee() with a name. (v2)

>+  popup.addEventListener("popupshown", function triggerPopupPopupShown() {
>+    popup.removeEventListener("popupshown", triggerPopupPopupShown, false);
Nit: PopupPopup looks cumbersome. Just use triggerPopupShown.

>+      aSubject.addEventListener("load", function observeASubjectLoad(aEvent) {
>+        aEvent.currentTarget.removeEventListener("load", observeASubjectLoad, false);
I don't think it makes sense to include "observe" here, just use aSubjectLoad.

>+    duplicateTab.linkedBrowser.addEventListener("load",
>+                                            function testTab2DupLBLoad(aEvent) {
Nit: incorrect wrapping.
I don't think it's possible to wrap this nicely, so don't bother.

>   browser.addEventListener("load", function loadListener(e) {
>-    browser.removeEventListener("load", arguments.callee, true);
>+    browser.removeEventListener("load", loadListener, true);
...
>     newTab.addEventListener("SSTabRestored", function tabRestored(e) {
>-      newTab.removeEventListener("SSTabRestored", arguments.callee, true);
>+      newTab.removeEventListener("SSTabRestored", tabRestored, true);
Wow, how did those sneak past their original review ;-)

Comment 6

[Edmund Wong (:ewong)]

Attachment: Replaced arguments.callee() with a name. (v3)

Fixed nits from comment #5.

Comment 7

[Edmund Wong (:ewong)]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/869e0e8618c3