Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#10208 closed defect (fixed)

memory leak in firebug lite code

Reported by: bill Owned by: James Burke
Priority: high Milestone: 1.4
Component: General Version: 1.4.0b
Keywords: Cc:
Blocked By: Blocking:


Since firebug adds symbols to window (specifically console), it should clean them up on page unload.

Not sure if that will break code though that calls console.log() on page unload.... themeTester.html seems to unload fine on IE6.

Also there's a dojo.connect() call that's better done as an dojo.addOnWindowUnload(), to avoid another leak.

Patch attached.

Attachments (1)

firebugLeaks.diff (718 bytes) - added by bill 12 years ago.

Download all attachments as: .zip

Change History (7)

Changed 12 years ago by bill

Attachment: firebugLeaks.diff added

comment:1 Changed 12 years ago by James Burke

Resolution: fixed
Status: newclosed

(In [20657]) Fixes #10208, leak patch from Bill for Firebug. \!strict

comment:2 Changed 12 years ago by bill

(In [20658]) One more leak fix for firebug lite, refs #10208 !strict.

comment:3 Changed 12 years ago by bill

Milestone: tbd1.4

comment:4 Changed 12 years ago by bill

(In [20662]) Code to deregister iframes/main window to avoid memory leaks on IE. Also, about mousedown listener, switched to using attachEvent()/attachListener() rather than dojo.connect() because the latter leaves a dojo._ieDispatcher() handler on dojo.doc (even after the dojo.disconnect() call), which causes a leak warning.

This change solves the leak, although could probably come up with a better design for the handle returned from registerWin()/registerIframe(). But that's an implementation detail. I was also conflicted about whether unregisterIframe() should take a handle or a reference to the iframe; ended up passing in a handle, like dojo.disconnect().

Refs #10208 !strict.

comment:5 Changed 12 years ago by bill

Oops, above checkin comment was for #10206.

comment:6 Changed 12 years ago by bill

[In [20920]) Rework selectOnClick code to get it working again. The trick is getting it to only select all the text on the first click. (Otherwise there would be no way to deselect the text.)

Since mousedown on a blurred field focuses it, it's tricky timing detecting when the mousedown causes focus versus when the field was already focused. (Plus detecting when a focus event was caused by tabbing into the field.) Note that the order of onmousedown and onfocus varies between IE and FF.

Seems possible to detect it though, in focus.js, after [20662]. So added a flag to _onFocus() callback that tells whether focus was caused by mousedown ("mouse") or not. (Using a string flag for possible future expansion via touch events etc.)

Fixes #10417, refs #10208, #8187 !strict.

Note: See TracTickets for help on using tickets.