give controls an events instance

With this patch, all controls get an events instance. Controls trigger "activate" and "deactivate" events. The control panel now takes advantage of these events, redrawing itself when appropriate. The navigation history control is now quite a bit simpler as this removes the need for onActivate and onDeactivate stuff.

Tests pass in FF/Safari.

give controls an events instance

Patch looks good to me. Makes things simpler (as can be seen in the example).

Why does the navigation history control have its own events property? And why that control uses this.map.events instead of this.events for registering event types?

Replying to elemoine:

Why does the navigation history control have its own events property? And why that control uses this.map.events instead of this.events for registering event types?

I misread your patch and thought you were adding the events property to the navigation history control. Thanks Chris for making me realize that.

Replying to elemoine:

Replying to elemoine:

Why does the navigation history control have its own events property? And why that control uses this.map.events instead of this.events for registering event types?

I misread your patch and thought you were adding the events property to the navigation history control. Thanks Chris for making me realize that.

So the patch looks good to me! Please commit.

(In [6167]) Giving all controls an events instance. You can now listen for activate and deactivate on any control. Panel controls do this to know when they should redraw. Navigation history control demonstrates the effect of this change. r=elemoine (closes #1346)

I am reopening this ticket since I have IE6 crashing on this change when closing the page. Specifically the this.events.destroy() line where it complains this.events is null.

On which line? Which control? The tests don't bring this up anymore, so it's something that isn't tested: can you tell which?

Line 168 in http://trac.openlayers.org/browser/trunk/openlayers/lib/OpenLayers/Control.js

If I alert the CLASS_NAME, then the one before the crash is OpenLayers.Control.Button.

I get:

Button
Button
NavigationHistory
Button again

Thanks; this is helpful.

I think I might know what's happening, the Map object tries to destroy the controls, but NavigationHistory also destroy next and prev .....

only destroy events if it exists

I'd rather the second.

Okay. I'm happy either way: I can't see any way in which it will change things :) Please commit

Who knows, someone might extend a control and nullify events themselves. The point is, with these ugly "sanity checks" we should limit how much we assume.

(In [6230]) Only destroy events if they are not already destroyed. This is relevant for controls with obligate controls. Since the map thinks it is in charge of destroying controls, and parent controls also destroy obligate controls, control.destroy ends up getting called twice. If someone wants to change the way this is handled, we should have a standard property that lets the map know that control.destroy is the responsibility of someone else. r=crschmidt (closes #1346)