Permalink
Browse files

Merge pull request #791 from sproutcore/team/tim-evans/segmented-view…

…-leak

Clean up item content observers on segmented views
  • Loading branch information...
2 parents b29c76c + 49c2fbf commit 2aecc88bef0de0ac410f04e4bad5043889694183 @tim-evans tim-evans committed Jun 11, 2012
Showing with 91 additions and 2 deletions.
  1. +63 −0 frameworks/desktop/tests/views/segmented/observers.js
  2. +28 −2 frameworks/desktop/views/segmented.js
@@ -0,0 +1,63 @@
+// ==========================================================================
+// Project: SproutCore - JavaScript Application Framework
+// Copyright: ©2006-2011 Strobe Inc. and contributors.
+// portions copyright @2011 Apple Inc.
+// License: Licensed under MIT license (see license.js)
+// ==========================================================================
+
+/*global module test htmlbody ok equals same stop start */
+
+var pane, view, item1, item2, item3;
+
+module("SC.SegmentedView observers", {
+ setup: function() {
+ SC.RunLoop.begin();
+ item1 = SC.Object.create({ value: "Item1" });
+ item2 = SC.Object.create({ value: "Item2" });
+ item3 = SC.Object.create({ value: "Item3" });
+ pane = SC.MainPane.create({
+ childViews: [
+ SC.SegmentedView.extend({
+ items: [item1, item2, item3],
+ itemTitleKey: 'value',
+ itemValueKey: null,
+ itemActionKey: 'action',
+ value: null,
+ allowsEmptySelection: YES,
+ layout: { height: 25, width: 400 }
+ })]
+ });
+ pane.append(); // make sure there is a layer...
+ SC.RunLoop.end();
+
+ view = pane.childViews[0];
+ },
+
+ teardown: function() {
+ pane.remove();
+ pane = view = null ;
+ }
+});
+
+test("Check that observers are removed properly", function() {
+ ok(item1.hasObserverFor('value'), 'Item1 should be observed');
+ ok(item2.hasObserverFor('value'), 'Item2 should be observed');
+ ok(item3.hasObserverFor('value'), 'Object2 should be observed');
+
+ SC.RunLoop.begin();
+ view.items.removeObject(item1);
+ SC.RunLoop.end();
+
+ ok(!item1.hasObserverFor('value'), 'Item1 should not be observed');
+ ok(item2.hasObserverFor('value'), 'Item2 should be observed');
+ ok(item3.hasObserverFor('value'), 'Item3 should be observed');
+
+ SC.RunLoop.begin();
+ view.items.removeObject(item3);
+ SC.RunLoop.end();
+
+ ok(!item1.hasObserverFor('value'), 'Item1 should not be observed');
+ ok(item2.hasObserverFor('value'), 'Item2 should be observed');
+ ok(!item3.hasObserverFor('value'), 'Item3 should not be observed');
+
+});
@@ -356,6 +356,7 @@ SC.SegmentedView = SC.View.extend(SC.Control,
var items = this.get('items') || [],
item,
localItem, // Used to avoid altering the original items
+ previousItem,
childViews = this.get('childViews'),
childView,
overflowView = childViews.lastObject(),
@@ -374,14 +375,27 @@ SC.SegmentedView = SC.View.extend(SC.Control,
// Remove unneeded segments from the end back
for (i = childViews.get('length') - 2; i >= items.get('length'); i--) {
childView = childViews.objectAt(i);
+ localItem = childView.get('value');
// If a selected childView has been removed then update our value
if (SC.isArray(value)) {
- value.removeObject(childView.get('value'));
- } else if (value === childView.get('value')) {
+ value.removeObject(localItem);
+ } else if (value === localItem) {
value = null;
}
+ // Remove observers from items we are losing off the end
+ if (localItem instanceof SC.Object) {
+
+ for (j = itemKeys.get('length') - 1; j >= 0; j--) {
+ itemKey = this.get(itemKeys.objectAt(j));
+
+ if (itemKey) {
+ localItem.removeObserver(itemKey, this, this.itemContentDidChange);
+ }
+ }
+ }
+
this.removeChild(childView);
}
@@ -413,6 +427,18 @@ SC.SegmentedView = SC.View.extend(SC.Control,
for (i = 0; i < items.get('length'); i++) {
localItem = items.objectAt(i);
childView = childViews.objectAt(i);
+ previousItem = childView.get('value');
+
+ if (previousItem instanceof SC.Object && !items.contains(previousItem)) {
+ // If the old item is no longer in the view, remove its observers
+ for (j = itemKeys.get('length') - 1; j >= 0; j--) {
+ itemKey = this.get(itemKeys.objectAt(j));
+
+ if (itemKey) {
+ previousItem.removeObserver(itemKey, this, this.itemContentDidChange);
+ }
+ }
+ }
// Skip null/undefined items (but don't skip empty strings)
if (SC.none(localItem)) continue;

0 comments on commit 2aecc88

Please sign in to comment.