feature.createPopup fails after popup.destroy

feature popup patch

I think this is:

• Not a regression
• A bad way to use the code, because you didn't create the popup: you destroying it seems destined for failure at some point.

This is no more clearly a 2.6 RC candidate than #1461, in my eyes.

Even more so than this being the wrong way to use the code, I just don't like the fix. Popups aren't aware of features. There's no reason for them to be, so I don't like this connection. I'm bumping to 2.7, unless there's some behavior here that changed in 2.6 from pre-2.6, but I don't see anything obvious.

The 'right' way to do this is to

1. Figure out why you need to destroy: are you just trying to hide the popup, perhaps?
2. If you need it, add a feature.destroyPopup() method.

Chris, the correct behaviour in our case is to just hide the popup. I agree that popups are not aware of features.

We (and probably others) have many popups in this particular app and so a potential memory management problem keeping all the popups around.

Since createPopup returns something, it seems likely that someone will look up the documentation for Popup and try destroying it, thinking that is the right thing to do.

I think we should add something to the docs in createPopup to say that the caller should *not* destroy the popup and add a feature.destroyPopup method. Um, there is already a destroyPopup method ... forget I said that, I suggest that this becomes a documentation bug to update createPopup method with a note that the caller should destroy the popup using destroyPopup.

Another question - would changing the popup's destroy function in feature.createPopup to call feature.destroyPopup be a bad thing?

this.popup.destroy = OpenLayers.Function.bind(this.destroyPopup, this);

This ticket should have been closed by r6820.