Opened 19 years ago

Closed 18 years ago

Last modified 18 years ago

#2142 closed defect (fixed)

[patch] Firefox JavaScript Console reports two warnings for admin script

Reported by: mike.capp@… Owned by: Adrian Holovaty
Component: contrib.admin Version:
Severity: minor Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

"function removeEvent does not always return a value" at contrib/admin/media/js/core.js line 25

MSDN docs state that detachEvent has no return, so we should presumably return true as we do for the removeEventListener path.

Incidentally (and somewhat offtopic) the addEvent in contrib/admin/media/js/admin/ordering.js appears to be an unnecessary duplicate of the one in core.js - the additional useCapture arg in this version isn't used by the only call to it.

variable e hides argument at contrib/admin/media/js/admin/DateTimeShortcuts.js line 220

The 'var' isn't necessary here; assigning to an undefined arg doesn't affect any variables with the same name in enclosing scopes. That is,

var e = 123;
function foo(e) { if (!e) e = 456; alert("in foo e is " + e); }
foo();
alert("outside foo e is " + e);

alerts 456 then 123. (Checked in FF, IE and Opera.)

Attachments (1)

ff_js_console_warnings.diff (952 bytes ) - added by mike.capp@… 19 years ago.
[patch] Sorry, forgot to put this in the ticket title

Download all attachments as: .zip

Change History (3)

by mike.capp@…, 19 years ago

Attachment: ff_js_console_warnings.diff added

[patch] Sorry, forgot to put this in the ticket title

comment:1 by anonymous, 18 years ago

Summary: Firefox JavaScript Console reports two warnings for admin script[patch] Firefox JavaScript Console reports two warnings for admin script

comment:2 by Mike Capp <mike.capp@…>, 18 years ago

Resolution: fixed
Status: newclosed

Fixed by Adrian in [3287] - not sure why it was still showing up as New here.

Note: See TracTickets for help on using tickets.
Back to Top