Permalink
Browse files

Fix widget focus listener logic, so that it doesn't leave one widget …

…in memory.

Fixes #2532198
See #2529690
  • Loading branch information...
1 parent 0aaf004 commit 4d3fe3fefd4b405adbeaa1d8639a248e00556b92 @sdesai sdesai committed Apr 25, 2012
Showing with 82 additions and 52 deletions.
  1. +1 −1 src/widget/HISTORY.md
  2. +15 −6 src/widget/js/Widget.js
  3. +66 −45 src/widget/tests/widget.html
View
@@ -1,7 +1,7 @@
Widget Change History
=====================
-3.6.0
+3.5.1
-----
* Cleaned up logic to detach document focus listener after last Widget is destroyed.
View
@@ -899,11 +899,14 @@ Y.extend(Widget, Y.Base, {
// Shared listener across all Widgets.
if (!focusHandle) {
focusHandle = Widget._hDocFocus = oDocument.on("focus", this._onDocFocus, this);
- focusHandle.listeners = 1;
- } else {
- focusHandle.listeners++;
+ focusHandle.listeners = {
+ count: 0
+ };
}
+ focusHandle.listeners[Y.stamp(this, true)] = true;
+ focusHandle.listeners.count++;
+
// Fix for Webkit:
// Document doesn't receive focus in Webkit when the user mouses
// down on it, so the "focused" attribute won't get set to the
@@ -921,14 +924,20 @@ Y.extend(Widget, Y.Base, {
_unbindDOM : function(boundingBox) {
var focusHandle = Widget._hDocFocus,
+ yuid = Y.stamp(this, true),
+ focusListeners,
mouseHandle = this._hDocMouseDown;
if (focusHandle) {
- if (focusHandle.listeners) {
- focusHandle.listeners--;
+
+ focusListeners = focusHandle.listeners;
+
+ if (focusListeners[yuid]) {
+ delete focusListeners[yuid];
+ focusListeners.count--;
}
- if (focusHandle.listeners === 0) {
+ if (focusListeners.count === 0) {
focusHandle.detach();
Widget._hDocFocus = null;
}
@@ -12,7 +12,7 @@
margin:0px;
min-height:0;
}
-
+
#testConsole .yui3-console-entry-fail .yui3-console-entry-cat {
background-color:red;
}
@@ -121,8 +121,7 @@
"testFocusOnChildFocus" : yeti && Y.UA.gecko,
"testFocusOnBoundingBox" : yeti && Y.UA.gecko,
"testMultiWidgetFocus" : yeti && Y.UA.gecko,
- "testWidgetOnFocus" : yeti && Y.UA.gecko,
- "testDetachFocusOnLastWidgetDestroy" : (testConsole !== undefined) // Can't test this if there are other Widget on the page
+ "testWidgetOnFocus" : yeti && Y.UA.gecko
}
},
@@ -459,7 +458,7 @@
"testVisible" : function() {
var w = this.createWidget({render:true});
var bb = w.get("boundingBox");
-
+
var className = w._cssPrefix + "-hidden";
Y.Assert.isTrue(w.get("visible"), "visible should be true by default");
@@ -856,7 +855,7 @@
Y.Assert.isTrue(gotFocus, "widget.on('focus') wasn't invoked");
},
-
+
"testDetachFocusOnLastWidgetDestroy" : function() {
var w1 = this.createWidget({
id : "foo",
@@ -868,11 +867,11 @@
render: true
});
- Y.Assert.areEqual(2, Y.Widget._hDocFocus.listeners);
+ Y.Assert.areEqual(2, Y.Widget._hDocFocus.listeners.count);
w1.destroy();
- Y.Assert.areEqual(1, Y.Widget._hDocFocus.listeners);
+ Y.Assert.areEqual(1, Y.Widget._hDocFocus.listeners.count);
w2.destroy();
@@ -1235,7 +1234,7 @@
w.destroy();
}
-
+
}, true);
suite.add(new Y.Test.Case(coreTests));
@@ -1575,7 +1574,7 @@
"afterDestroy"
];
- w = new MyLifecycleWidget({
+ var w = new MyLifecycleWidget({
on: {
init: function() {
this.__test.push("onInit");
@@ -1628,7 +1627,7 @@
"afterDestroy"
];
- w = new MyLifecycleWidget({
+ var w = new MyLifecycleWidget({
on: {
init: function() {
this.__test.push("onInit");
@@ -1902,17 +1901,21 @@
var w = new Y.Widget();
Y.Assert.isNull( w.getSkinName() );
+
+ w.destroy();
},
"getSkinName should return name from BB if available": function () {
var bb = Y.Node.create( '<div class="yui3-skin-foo"><div></div></div>' ),
cb = bb.one( 'div' ),
w = new Y.Widget( {
- boundingBox: bb,
- contentBox: cb
- } );
+ boundingBox: bb,
+ contentBox: cb
+ } );
Y.Assert.areEqual( "foo", w.getSkinName() );
+
+ w.destroy();
},
"getSkinName should return name from body or null": function () {
@@ -2018,13 +2021,13 @@
}
}
}));
-
+
suite.add(new Y.Test.Case({
name:"UI Events",
testSingleSimple: function() {
-
+
var w, h, cb,
actualEvents = [],
expectedEvents = ["widget:click"];
@@ -2045,10 +2048,10 @@
w.destroy();
},
-
- testSingleComplex: function() {
-
- var w, h, cb
+
+ testSingleComplex : function() {
+
+ var w, h, cb,
actualEvents = [],
expectedEvents = ["widget:render",
"widget:renderedChange",
@@ -2063,6 +2066,7 @@
"widget:click"];
w = new Y.Widget();
+
cb = w.get("contentBox");
cb.append("<p class='et'>Some Content For My Widget</p>");
@@ -2074,11 +2078,11 @@
"render": h,
"renderedChange": h
});
-
+
w.on("widget:mouseup", h);
w.on("foo|widget:mouseup", h);
w.on("mouseup", h);
-
+
w.after("widget:mouseup", h);
w.after("foo|widget:mouseup", h);
w.after("mouseup", h);
@@ -2087,68 +2091,70 @@
"mousedown" : h,
"render" : h
});
-
+
w.render();
cb.one(".et").simulate("mousedown");
cb.one(".et").simulate("mouseup");
cb.one(".et").simulate("click");
Y.ArrayAssert.itemsAreEqual(expectedEvents, actualEvents);
-
+
w.destroy();
},
testNested: function() {
-
+
var outer = new Y.Widget();
var inner = new Y.Widget();
var ocb = outer.get('contentBox');
var icb = inner.get('contentBox');
-
+
var expectedEvents = ["outerClick", "innerClick", "outerClick"];
var actualEvents = [];
-
+
outer.render();
ocb.setContent("<span class='oet'>Outer Content</span>");
-
+
inner.render(ocb);
icb.setContent("<span class='iet'>Inner Content</span>");
-
+
inner.after('click', function() {actualEvents.push("innerClick");});
outer.after('click', function() {actualEvents.push("outerClick");});
-
+
// Only outer
ocb.one(".oet").simulate("click");
-
+
// One Inner, One Outer
ocb.one(".iet").simulate("click");
-
+
Y.ArrayAssert.itemsAreEqual(expectedEvents, actualEvents);
-
+
outer.destroy();
inner.destroy();
},
-
+
testMultipleInstances : function() {
-
+
var actualEvents = [],
expectedEvents = ["clickOuter", "clickInner", "clickOuter"],
+ Y1,
+ Y2,
w1,
w2;
-
- YUI().use('widget', function (Y) {
-
- w1 = new Y.Widget({render:true});
+
+ Y1 = YUI().use('widget', function (Y1) {
+
+ w1 = new Y1.Widget({render:true});
w1.get('contentBox').append('<div class="w2-container"></div><span class="miouter">Outer</span>');
w1.on('click', function (e) {
actualEvents.push("clickOuter");
});
- YUI().use('widget', function (Y1) {
+ Y2 = YUI().use('widget', function (Y2) {
- w2 = new Y1.Widget({render:".w2-container"});
+ w2 = new Y2.Widget({render:".w2-container"});
w2.get('contentBox').append('<span class="miinner">Inner</span>');
w2.on('click', function (e) {
@@ -2159,11 +2165,14 @@
Y.Node.one(".miouter").simulate("click"); // only outer, once.
Y.Node.one(".miinner").simulate("click"); // inner, bubbled to outer (once each, without JS errors)
-
+
Y.ArrayAssert.itemsAreEqual(expectedEvents, actualEvents);
-
+
w1.destroy();
w2.destroy();
+
+ Y1.destroy();
+ Y2.destroy();
},
testPublishDefaultFn : function() {
@@ -2187,9 +2196,9 @@
w.destroy();
}
-
+
}));
-
+
suite.add(new Y.Test.Case({
name:"clone",
@@ -2225,6 +2234,10 @@
Y.Assert.isTrue(c1 instanceof Y.Widget);
Y.Assert.isTrue(c2 instanceof Y.Widget);
Y.Assert.isTrue(c3 instanceof Y.Widget);
+
+ a.destroy();
+ b.destroy();
+ c.destroy();
},
testWidgetHashClone : function() {
@@ -2249,6 +2262,10 @@
Y.Assert.isTrue(o3.a instanceof Y.Widget);
Y.Assert.isTrue(o3.b instanceof Y.Widget);
Y.Assert.isTrue(o3.c instanceof Y.Widget);
+
+ a.destroy();
+ b.destroy();
+ c.destroy();
},
testBaseClone : function() {
@@ -2323,6 +2340,10 @@
runButton.set("value", "Run Tests");
runButton.set("disabled", false).on("click", function() {
if (!testConsole) {
+
+ // This test should be ignored if Console is instantiated.
+ coreTests._should.ignore.testDetachFocusOnLastWidgetDestroy = true;
+
testConsole = new Y.Console({
id:"testConsole",
width:"100%",
@@ -2344,7 +2365,7 @@
});
Y.Test.Runner.run();
-
+
});
</script>
</body>

0 comments on commit 4d3fe3f

Please sign in to comment.