OpenLayers OpenLayers

Ticket #642 (closed bug: fixed)

Opened 2 years ago

Last modified 10 months ago

navigation control needs destroy

Reported by: tschaub Assigned to: tschaub
Priority: minor Milestone: 2.6 Release
Component: Control.Navigation Version:
Keywords: Cc:
State: Complete

Description

Not all of the controls have their own destroy(). The superclass destroy isn't sufficient in all cases. Same for handlers.

Uncertain if this needs to go in 2.4, so I'm leaving the milestone off.

Attachments

navigation.patch (0.8 kB) - added by crschmidt on 01/23/08 21:31:58.

Change History

05/23/07 23:18:21 changed by crschmidt

  • milestone set to 2.5 Release.

07/16/07 00:38:29 changed by crschmidt

  • milestone changed from 2.5 Release to Maintenance.

12/17/07 18:51:22 changed by tschaub

  • status changed from new to closed.
  • state changed.
  • resolution set to wontfix.

Taking this out of my queue. Still a good idea, I just don't want too many tickets without patches to consider. Since I reported this, I'm reserving the right to close it.

01/08/08 14:34:31 changed by tschaub

  • status changed from closed to reopened.
  • resolution deleted.

Ok, seems important enough to reopen this.

For example:

  • go to http://openlayers.org
  • open firebug
  • run the following
    c = map.controls[0];
    map.destroy();
    
  • then peruse things like
    c.dragPan.handler.map.layerContainerDiv
    

01/08/08 14:34:51 changed by tschaub

  • milestone changed from Maintenance to 2.6 Release.

01/23/08 21:31:48 changed by crschmidt

  • state set to Needs More Work.

In general, I think the handlers are in good shape. They clean up after themseles without a problem.

In general, controls which use a handler with the variable name 'handler' are in good shape. Some problems, but nothing major.

The problem seems to be in the case where controls use *more* than one handler, and are therefore responsible for cleaning up after themselves. This applies to the Navigation control, which has no destroy() method at all.

About to attach a patch which takes care of at least some of this problem. I have no idea if you (tim) are looking for more serious work in a general sense. I still feel that the only way, right now, to address this problem systematically is to get a working copy of 'Drip' running, and go start messing with the things you care about. It's very easy to see quickly where memory leaks are happening, and they're typically easy to fix, but doing it by evaluating the controls manually seems likely to either be a pain in the butt, or miss something.

01/23/08 21:31:58 changed by crschmidt

  • attachment navigation.patch added.

01/24/08 19:30:46 changed by tschaub

  • state changed from Needs More Work to Commit.
  • component changed from Control to Control.Navigation.
  • summary changed from add destroy to controls and handlers to navigation control needs destroy.

I think this should go in. There are still plenty of issues, but I'll just keep them in mind (you can still explore c.wheelHandler.evt.element and such things if you have a reference to the nav control). I'll make other tickets as things come up.

01/24/08 20:36:22 changed by crschmidt

  • status changed from reopened to closed.
  • state changed from Commit to Complete.
  • resolution set to fixed.

applied in r5891

01/25/08 11:58:17 changed by euzuro

This ticket is a duplication of #1289