OpenLayers OpenLayers

Ticket #1649 (closed feature: fixed)

Opened 4 months ago

Last modified 3 months ago

Layer.Vector.removeMap must deactivate the strategies

Reported by: elemoine Assigned to: elemoine
Priority: major Milestone: 2.7 Release
Component: Layer.Vector Version: SVN
Keywords: Cc:
State: Complete

Description

Currently Layer.Vector activates its strategies in setMap, meaning that the strategies will automatically be activated when the vector layer is added to the map. This is good so far. What's not good is that when the layer is removed from the map the strategies aren't deactivated, which is particularly bad because the strategies' listeners aren't unregistered.

Attachments

patch-1649-r7658-A0.diff (5.1 kB) - added by elemoine on 08/01/08 08:46:25.
patch-1649-r7658-B0.diff (7.6 kB) - added by elemoine on 08/01/08 09:30:23.
patch-1649-r7658-C0.diff (10.9 kB) - added by fredj on 08/04/08 10:00:25.
Same patch but add unit tests in Vector.html to test the autoDestroy. Also fix the typo spotted in #1659
patch-1649-r7658-D0.diff (8.3 kB) - added by elemoine on 08/05/08 09:18:24.

Change History

08/01/08 08:46:25 changed by elemoine

  • attachment patch-1649-r7658-A0.diff added.

08/01/08 08:50:26 changed by elemoine

  • state set to Review.

patch-1649-r7658-A0.diff fixes the problem and adds two things:

  • makes activate()/deactivate() return false if the strategy was already activated/deactivated, true otherwise
  • adds the property autoActivate to Strategy. If true the layer takes care of activating and deactivating the strategy in setMap and removeMap, respectively. If false, the layer isn't responsible for that. As already discussed this Tim this is useful if the user wishes full control on his strategies.

Tests pass on FF2 and FF3.

08/01/08 09:15:00 changed by elemoine

  • state changed from Review to Needs More Work.
  • version changed from 2.6 to SVN.

Actually, I'd like to go one step further and introduce an autoDestroy property to Strategy and Protocol.

08/01/08 09:30:23 changed by elemoine

  • attachment patch-1649-r7658-B0.diff added.

08/01/08 09:30:59 changed by elemoine

  • state changed from Needs More Work to Review.

See patch-1649-r7658-B0.diff. Please review.

08/04/08 10:00:25 changed by fredj

  • attachment patch-1649-r7658-C0.diff added.

Same patch but add unit tests in Vector.html to test the autoDestroy. Also fix the typo spotted in #1659

08/05/08 09:18:24 changed by elemoine

  • attachment patch-1649-r7658-D0.diff added.

08/05/08 09:20:49 changed by elemoine

patch-1649-r7658-D0.diff applies to current trunk (r7658). In particular, a change to the existing Fixed strategy has been added. I'd really like that we consider this patch before committing new concrete strategies. Please review.

08/05/08 09:38:26 changed by fredj

  • state changed from Review to Commit.

In Strategy.js, there is a typo in ND comments for activate and deactivate (the Returns comment).

Once done, please commit.

08/05/08 09:59:12 changed by elemoine

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

(In [7708]) Layer.Vector.removeMap must deactivate the strategies, r=fredj (closes #1649)

09/05/08 02:14:42 changed by euzuro

  • component changed from general to Layer.Vector.