fix(jqLite): have same expando format as jQuery

angular · May 17, 2012 · acf095d · acf095d
1 parent 301d8f2
commit acf095d
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 43 deletions.
diff --git a/src/jqLite.js b/src/jqLite.js
@@ -186,8 +186,8 @@ function JQLiteDealoc(element){
 }
 
 function JQLiteUnbind(element, type, fn) {
-  var events = JQLiteData(element, 'events'),
-      handle = JQLiteData(element, 'handle');
+  var events = JQLiteExpandoStore(element, 'events'),
+      handle = JQLiteExpandoStore(element, 'handle');
 
   if (!handle) return; //no listeners registered
 
@@ -207,44 +207,56 @@ function JQLiteUnbind(element, type, fn) {
 }
 
 function JQLiteRemoveData(element) {
-  var cacheId = element[jqName],
-      cache = jqCache[cacheId];
+  var expandoId = element[jqName],
+      expandoStore = jqCache[expandoId];
 
-  if (cache) {
-    if (cache.handle) {
-      cache.events.$destroy && cache.handle({}, '$destroy');
+  if (expandoStore) {
+    if (expandoStore.handle) {
+      expandoStore.events.$destroy && expandoStore.handle({}, '$destroy');
       JQLiteUnbind(element);
     }
-    delete jqCache[cacheId];
+    delete jqCache[expandoId];
     element[jqName] = undefined; // ie does not allow deletion of attributes on elements.
   }
 }
 
-function JQLiteData(element, key, value) {
-  var cacheId = element[jqName],
-      cache = jqCache[cacheId || -1];
+function JQLiteExpandoStore(element, key, value) {
+  var expandoId = element[jqName],
+      expandoStore = jqCache[expandoId || -1];
 
   if (isDefined(value)) {
-    if (!cache) {
-      element[jqName] = cacheId = jqNextId();
-      cache = jqCache[cacheId] = {};
+    if (!expandoStore) {
+      element[jqName] = expandoId = jqNextId();
+      expandoStore = jqCache[expandoId] = {};
     }
-    cache[key] = value;
+    expandoStore[key] = value;
+  } else {
+    return expandoStore && expandoStore[key];
+  }
+}
+
+function JQLiteData(element, key, value) {
+  var data = JQLiteExpandoStore(element, 'data'),
+      isSetter = isDefined(value),
+      keyDefined = !isSetter && isDefined(key),
+      isSimpleGetter = keyDefined && !isObject(key);
+
+  if (!data && !isSimpleGetter) {
+    JQLiteExpandoStore(element, 'data', data = {});
+  }
+
+  if (isSetter) {
+    data[key] = value;
   } else {
-    if (isDefined(key)) {
-      if (isObject(key)) {
-        if (!cacheId) element[jqName] = cacheId = jqNextId();
-        jqCache[cacheId] = cache = (jqCache[cacheId] || {});
-        extend(cache, key);
+    if (keyDefined) {
+      if (isSimpleGetter) {
+        // don't create data in this case.
+        return data && data[key];
       } else {
-        return cache ? cache[key] : undefined;
+        extend(data, key);
       }
     } else {
-      if (!cacheId) element[jqName] = cacheId = jqNextId();
-
-      return cache
-          ? cache
-          : cache = jqCache[cacheId] = {};
+      return data;
     }
   }
 }
@@ -583,11 +595,11 @@ forEach({
   dealoc: JQLiteDealoc,
 
   bind: function bindFn(element, type, fn){
-    var events = JQLiteData(element, 'events'),
-        handle = JQLiteData(element, 'handle');
+    var events = JQLiteExpandoStore(element, 'events'),
+        handle = JQLiteExpandoStore(element, 'handle');
 
-    if (!events) JQLiteData(element, 'events', events = {});
-    if (!handle) JQLiteData(element, 'handle', handle = createEventHandler(element, events));
+    if (!events) JQLiteExpandoStore(element, 'events', events = {});
+    if (!handle) JQLiteExpandoStore(element, 'handle', handle = createEventHandler(element, events));
 
     forEach(type.split(' '), function(type){
       var eventFns = events[type];

diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js
@@ -353,6 +353,18 @@ describe('ngMock', function() {
       return keys.sort();
     }
 
+    function browserTrigger(element, eventType) {
+      element = element[0];
+      if (document.createEvent) {
+        var event = document.createEvent('MouseEvents');
+        event.initMouseEvent(eventType, true, true, window, 0, 0, 0, 0, 0, false, false,
+          false, false, 0, element);
+        element.dispatchEvent(event);
+      } else {
+        element.fireEvent('on' + eventType);
+      }
+    }
+
     it('should remove data', function() {
       expect(angular.element.cache).toEqual({});
       var div = angular.element('<div></div>');
@@ -364,17 +376,29 @@ describe('ngMock', function() {
 
     it('should deregister event handlers', function() {
       expect(keys(angular.element.cache)).toEqual([]);
-
+      var log = '';
       var div = angular.element('<div></div>');
 
-      div.bind('click', angular.noop);
-      div.bind('mousemove', angular.noop);
-      div.data('some', 'data');
-      expect(keys(angular.element.cache).length).toBe(1);
+      // crazy IE9 requires div to be connected to render DOM for click event to work
+      // mousemove works even when not connected. This is a heisen-bug since stepping
+      // through the code makes the test pass. Viva IE!!!
+      angular.element(document.body).append(div)
+
+      div.bind('click', function() { log += 'click1;'});
+      div.bind('click', function() { log += 'click2;'});
+      div.bind('mousemove', function() { log += 'mousemove;'});
+
+      browserTrigger(div, 'click');
+      browserTrigger(div, 'mousemove');
+      expect(log).toEqual('click1;click2;mousemove;');
+      log = '';
 
       angular.mock.clearDataCache();
+
+      browserTrigger(div, 'click');
+      browserTrigger(div, 'mousemove');
+      expect(log).toEqual('');
       expect(keys(angular.element.cache)).toEqual([]);
-      expect(div.data('some')).toBeUndefined();
 
       div.remove();
     });

diff --git a/test/testabilityPatch.js b/test/testabilityPatch.js
@@ -40,9 +40,14 @@ afterEach(function() {
 
   // complain about uncleared jqCache references
   var count = 0;
-  forEachSorted(jqCache, function(value, key){
-    count ++;
-    forEach(value, function(value, key){
+
+  // This line should be enabled as soon as this bug is fixed: http://bugs.jquery.com/ticket/11775
+  //var cache = jqLite.cache;
+  var cache = JQLite.cache;
+
+  forEachSorted(cache, function(expando, key){
+    forEach(expando.data, function(value, key){
+      count ++;
       if (value.$element) {
         dump('LEAK', key, value.$id, sortedHtml(value.$element));
       } else {
@@ -57,20 +62,25 @@ afterEach(function() {
 
 
 function dealoc(obj) {
+  var jqCache = jqLite.cache;
   if (obj) {
     if (isElement(obj)) {
-      var element = obj;
-      if (element.nodeName) element = jqLite(element);
-      if (element.dealoc) element.dealoc();
+      cleanup(jqLite(obj));
     } else {
       for(var key in jqCache) {
         var value = jqCache[key];
-        if (value.$scope == obj) {
+        if (value.data && value.data.$scope == obj) {
           delete jqCache[key];
         }
       }
     }
+  }
 
+  function cleanup(element) {
+    element.unbind().removeData();
+    for ( var i = 0, children = element.children() || []; i < children.length; i++) {
+      cleanup(jqLite(children[i]));
+    }
   }
 }