From 60e39885b37c7c6a8a5215b6e968cbf7101fe358 Mon Sep 17 00:00:00 2001 From: Mark Finkle Date: Thu, 15 Jan 2009 13:47:55 -0500 Subject: [PATCH] Bug 473156: fuelIEvents.removeListener removes all listeners for an event, r=dtownsend --- browser/fuel/test/browser_ApplicationPrefs.js | 7 +++++++ browser/fuel/test/browser_Browser.js | 17 +++++++++++------ toolkit/components/exthelper/extApplication.js | 4 ++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/browser/fuel/test/browser_ApplicationPrefs.js b/browser/fuel/test/browser_ApplicationPrefs.js index 42d5e7a72f4..9f1884e2e3d 100644 --- a/browser/fuel/test/browser_ApplicationPrefs.js +++ b/browser/fuel/test/browser_ApplicationPrefs.js @@ -158,7 +158,12 @@ function onPrefChange(evt) { is(evt.data, testdata.dummy, "Check 'Application.prefs.set' fired a change event"); Application.prefs.events.removeListener("change", onPrefChange); + // We are removing the old listener after adding the new listener so we can test that + // removing a listener does not remove all listeners + Application.prefs.get("fuel.fuel-test").events.addListener("change", onPrefChangeDummy); Application.prefs.get("fuel.fuel-test").events.addListener("change", onPrefChange2); + Application.prefs.get("fuel.fuel-test").events.removeListener("change", onPrefChangeDummy); + Application.prefs.setValue("fuel.fuel-test", "change event2"); } @@ -168,3 +173,5 @@ function onPrefChange2(evt) { finish(); } + +function onPrefChangeDummy(evt) { } diff --git a/browser/fuel/test/browser_Browser.js b/browser/fuel/test/browser_Browser.js index 73334f0d631..7da7da721f7 100644 --- a/browser/fuel/test/browser_Browser.js +++ b/browser/fuel/test/browser_Browser.js @@ -35,18 +35,20 @@ function test() { gPageA.events.removeListener("load", onPageAFirstLoad); gPageB = activeWin.open(url("chrome://mochikit/content/browser/browser/fuel/test/ContentB.html")); - gPageB.events.addListener("load", function() { - executeSoon(afterOpen); - }); + gPageB.events.addListener("load", delayAfterOpen); gPageB.focus(); is(activeWin.tabs.length, 3, "Checking length of 'Browser.tabs' after opening a second additional tab"); is(activeWin.activeTab.index, gPageB.index, "Checking 'Browser.activeTab' after setting focus"); } + function delayAfterOpen() { + executeSoon(afterOpen); + } + // need to wait for the url's to be refreshed during the load function afterOpen(event) { - gPageB.events.removeListener("load", afterOpen); + gPageB.events.removeListener("load", delayAfterOpen); is(gPageA.uri.spec, "chrome://mochikit/content/browser/browser/fuel/test/ContentA.html", "Checking 'BrowserTab.uri' after opening"); is(gPageB.uri.spec, "chrome://mochikit/content/browser/browser/fuel/test/ContentB.html", "Checking 'BrowserTab.uri' after opening"); @@ -60,6 +62,9 @@ function test() { is(test1.innerHTML, "A", "Checking content of element in content DOM"); // test moving tab + is(gTabMoveCount, 0, "Checking initial tab move count"); + + // move the tab gPageA.moveToEnd(); is(gPageA.index, 2, "Checking index after moving tab"); @@ -89,7 +94,7 @@ function test() { }); // test loading new content with a frame into a tab - // the event will be checked in afterClose + // the event will be checked in onPageBLoadComplete gPageB.events.addListener("load", onPageBLoadWithFrames); gPageB.load(url("chrome://mochikit/content/browser/browser/fuel/test/ContentWithFrames.html")); } @@ -104,7 +109,7 @@ function test() { is(gPageLoadCount, 1, "Checking load count after loading new content with a frame"); // test loading new content into a tab - // the event will be checked in onPageLoad + // the event will be checked in onPageASecondLoad gPageA.events.addListener("load", onPageASecondLoad); gPageA.load(url("chrome://mochikit/content/browser/browser/fuel/test/ContentB.html")); } diff --git a/toolkit/components/exthelper/extApplication.js b/toolkit/components/exthelper/extApplication.js index 28a6cb45b81..5df8d4e76cf 100644 --- a/toolkit/components/exthelper/extApplication.js +++ b/toolkit/components/exthelper/extApplication.js @@ -128,7 +128,7 @@ Events.prototype = { this._listeners = this._listeners.filter(hasFilter); function hasFilter(element) { - return element.event != aEvent && element.listener != aListener; + return (element.event != aEvent) || (element.listener != aListener); } }, @@ -696,7 +696,7 @@ extApplication.prototype = { os.notifyObservers(cancelQuit, "quit-application-requested", null); if (cancelQuit.data) return false; // somebody canceled our quit request - + let appStartup = Components.classes['@mozilla.org/toolkit/app-startup;1'] .getService(Components.interfaces.nsIAppStartup); appStartup.quit(aFlags);