From acf095d1783e30e750d046ef24e81b5a0a31fbd4 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Wed, 9 May 2012 19:27:15 -0400 Subject: [PATCH] fix(jqLite): have same expando format as jQuery --- src/jqLite.js | 72 +++++++++++++++++++------------- test/ngMock/angular-mocksSpec.js | 36 +++++++++++++--- test/testabilityPatch.js | 24 +++++++---- 3 files changed, 89 insertions(+), 43 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index 516438ff686c..1868f565aa0b 100644 --- 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 index 88946ab9991c..469df91e8e45 100644 --- 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('
'); @@ -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.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 index f033dda2d7c7..ac22c72c05cd 100644 --- 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])); + } } }