patch for #621 - assign items in the elements hash table a unique cacheID instead of relying on the element's actual 'id' property. thx for thorough review sde

git-svn-id: http://svn.openlayers.org/trunk/openlayers@2993 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf

lib/OpenLayers/Events.js

@@ -10,7 +10,8 @@
  */
 OpenLayers.Event = {
 
-    /** A hashtable cache of the event observers, keyed by element.id
+    /** A hashtable cache of the event observers.
+     *   Keyed by element._eventCacheID
      * 
      * @type Object
      */
@@ -119,13 +120,24 @@ OpenLayers.Event = {
             this.observers = new Object();
         }
 
+        //if not already assigned, make a new unique cache ID
+        if (!element._eventCacheID) {
+            var idPrefix = "eventCacheID_";
+            if (element.id) {
+                idPrefix = element.id + "_" + idPrefix;
+            }
+            element._eventCacheID = OpenLayers.Util.createUniqueID(idPrefix);
+        }
+
+        var cacheID = element._eventCacheID;
+
         //if there is not yet a hash entry for this element, add one
-        if (!this.observers[element.id]) {
+        if (!this.observers[cacheID]) {
-            this.observers[element.id] = new Array();
+            this.observers[cacheID] = new Array();
         }
 
         //add a new observer to this element's list
-        this.observers[element.id].push({
+        this.observers[cacheID].push({
             'element': element,
             'name': name,
             'observer': observer,
@@ -147,23 +159,28 @@ OpenLayers.Event = {
      * @param {DOMElement || String} elementParam
      */
     stopObservingElement: function(elementParam) {
+        var element = OpenLayers.Util.getElement(elementParam);
+        var cacheID = element._eventCacheID;
 
-        var elementId = (typeof elementParam == "string") ? elementParam 
+        this._removeElementObservers(OpenLayers.Event.observers[cacheID]);
-                            : OpenLayers.Util.getElement(elementParam).id;
+    },
 
-        var elementObservers = OpenLayers.Event.observers[elementId];
+    /**
+     * @private
+     * 
+     * @param {Array(Object)} elementObservers Array of (element, name, 
+     *                                         observer, usecapture) objects, 
+     *                                         taken directly from hashtable
+     */
+    _removeElementObservers: function(elementObservers) {
         if (elementObservers) {
-            var i=0;
+            for(var i = elementObservers.length-1; i >= 0; i--) {
-            while(i < elementObservers.length) {
+                var entry = elementObservers[i];
-                var entry = elementObservers[0];
                 var args = new Array(entry.element,
                                      entry.name,
                                      entry.observer,
                                      entry.useCapture);
                 var removed = OpenLayers.Event.stopObserving.apply(this, args);
-                if (!removed) {
-                    i++;
-                }
             }
         }
     },
@@ -181,8 +198,7 @@ OpenLayers.Event = {
         useCapture = useCapture || false;
     
         var element = OpenLayers.Util.getElement(elementParam);
-        var elementId = (typeof elementParam == "string") ? elementParam
+        var cacheID = element._eventCacheID;
-                                                          : element.id;
 
         if (name == 'keypress') {
             if ( navigator.appVersion.match(/Konqueror|Safari|KHTML/) || 
@@ -193,7 +209,7 @@ OpenLayers.Event = {
 
         // find element's entry in this.observers cache and remove it
         var foundEntry = false;
-        var elementObservers = OpenLayers.Event.observers[elementId];
+        var elementObservers = OpenLayers.Event.observers[cacheID];
         if (elementObservers) {
     
             // find the specific event type in the element's list
@@ -207,7 +223,7 @@ OpenLayers.Event = {
     
                     elementObservers.splice(i, 1);
                     if (elementObservers.length == 0) {
-                        delete OpenLayers.Event.observers[element.id];
+                        delete OpenLayers.Event.observers[cacheID];
                     }
                     foundEntry = true;
                     break; 
@@ -230,8 +246,10 @@ OpenLayers.Event = {
      */
     unloadCache: function() {
         if (OpenLayers.Event.observers) {
-            for (var elementId in OpenLayers.Event.observers) {
+            for (var cacheID in OpenLayers.Event.observers) {
-                OpenLayers.Event.stopObservingElement.apply(this, [elementId]);
+                var elementObservers = OpenLayers.Event.observers[cacheID];
+                OpenLayers.Event._removeElementObservers.apply(this, 
+                                                           [elementObservers]);
             }
             OpenLayers.Event.observers = false;
         }

tests/test_Events.html

@@ -202,14 +202,15 @@
         t.eq(a, 5, "if Events has no object set and an event is registered also with no object, triggerEvent() calls it without trying to set the context to null");        
     }
 
-    function test_05_Event_destroy (t) {
+    function test_05_Events_destroy (t) {
         t.plan(2);
 
         var div   = OpenLayers.Util.getElement('test');
         var obj   = {};
         var events = new OpenLayers.Events(obj, div);
+
         // +1 because of blocking dragstart in attachToElement()
-        t.eq(OpenLayers.Event.observers["test"].length,
+        t.eq(OpenLayers.Event.observers[div._eventCacheID].length,
              OpenLayers.Events.prototype.BROWSER_EVENTS.length + 1,
              "construction creates new arrayin hash, registers appropriate events");
              
@@ -219,11 +220,98 @@
              "destruction removes the event observer from hash");
     }
     
+    function test_06_Event(t) {
+        t.plan(24);
+        
+        var div   = OpenLayers.Util.getElement('test');
+        var name = "mouseover";
+        var func = function() {};
+
+      //1st elem 1st listener        
+        OpenLayers.Event.observe(div, name, func);
+
+        var cacheID = div._eventCacheID;
+        t.ok(cacheID, "element given new cache id");
+        
+        var elementObservers = OpenLayers.Event.observers[cacheID];
+        
+        t.ok(elementObservers, "new cache bucket made for event");
+        t.eq(elementObservers.length, 1, "one listener registered");        
+        
+        var listener = elementObservers[0];
+        
+        t.ok(listener.element == div, "element registered");
+        t.eq(listener.name, name, "name registered");
+        t.ok(listener.observer == func, "function registered");
+        t.eq(listener.useCapture, false, "useCapture defaults to false");
+
+      //1st elem 2nd listener        
+        name = "mouseout";
+        var newFunc = function() {};
+      
+        OpenLayers.Event.observe(div, name, newFunc, true);
+        var newCacheID = div._eventCacheID;
+        t.eq(newCacheID, cacheID, "element's cache id not overridden");
+        
+        t.eq(elementObservers.length, 2, "listener added to existing bucket");        
+        
+        var listener = elementObservers[1];
+        
+        t.ok(listener.element == div, "element registered");
+        t.eq(listener.name, name, "name registered");
+        t.ok(listener.observer == newFunc, "function registered");
+        t.eq(listener.useCapture, true, "useCapture correctly registered");
+
+      //2st elem 1st listener        
+        div = OpenLayers.Util.getElement('test2'); 
+        OpenLayers.Event.observe(div, name, func);
+
+        var cacheID = div._eventCacheID;
+        t.ok(cacheID, "new element given new cache id");
+        t.ok(cacheID != newCacheID, "new cache id is unique");
+        
+        elementObservers = OpenLayers.Event.observers[cacheID];
+        
+        t.ok(elementObservers, "new cache bucket made for event");
+        t.eq(elementObservers.length, 1, "one listener registered");        
+        
+        var listener = elementObservers[0];
+        
+        t.ok(listener.element == div, "element registered");
+        t.eq(listener.name, name, "name registered");
+        t.ok(listener.observer == func, "function registered");
+        t.eq(listener.useCapture, false, "useCapture defaults to false");
+
+      //stopObservingElement by element
+        OpenLayers.Event.stopObservingElement(div);
+        elementObservers = OpenLayers.Event.observers[cacheID];
+        t.ok(elementObservers == null, "stopObservingElement by elem works");
+
+      //stopObservingElement by id
+        OpenLayers.Event.stopObservingElement("test");
+        elementObservers = OpenLayers.Event.observers[newCacheID];
+        t.ok(elementObservers == null, "stopObservingElement by id works");
+
+
+      //unloadCache by element
+        OpenLayers.Event.observe(div, name, func);
+        
+        OpenLayers.Event.unloadCache();
+
+        elementObservers = OpenLayers.Event.observers[cacheID];
+        t.ok(elementObservers == null, "stopObservingElement by elem works");
+
+        
+    }
+
+
+   
   // -->
   </script>
 </head>
 <body>
     <div id="map" style="width: 1024px; height: 512px;"/>
     <div id="test"></div>
+    <div id="test2"></div>
 </body>
 </html>