From 19d955cc32123bfc95ec6272113b0d6573616bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A3o=20Gottwald?= Date: Mon, 18 Feb 2013 22:06:03 +0100 Subject: [PATCH] Bug 839445 - PopupNotifications need to know about location changes in background tabs. r=gavin --HG-- extra : rebase_source : 3e12347e0e9eaa257343c941e37e3c417be94542 --- browser/base/content/browser.js | 18 +++----- .../content/test/browser_popupNotification.js | 44 +++++++++++++++---- toolkit/content/PopupNotifications.jsm | 23 ++++++---- 3 files changed, 56 insertions(+), 29 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 2097dd673428..0a5642fb6438 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -4341,18 +4341,6 @@ var XULBrowserWindow = { SocialShareButton.updateShareState(); } - // Filter out anchor navigation, history.push/pop/replaceState and - // tab switches. - if (aRequest) { - // Only need to call locationChange if the PopupNotifications object - // for this window has already been initialized (i.e. its getter no - // longer exists) - // XXX bug 839445: We never tell PopupNotifications about location - // changes in background tabs. - if (!__lookupGetter__("PopupNotifications")) - PopupNotifications.locationChange(); - } - // Show or hide browser chrome based on the whitelist if (this.hideChromeForLocation(location)) { document.documentElement.setAttribute("disablechrome", "true"); @@ -4776,6 +4764,12 @@ var TabsProgressListener = { aBrowser._clickToPlayPluginsActivated = new Map(); aBrowser._clickToPlayAllPluginsActivated = false; aBrowser._pluginScriptedState = gPluginHandler.PLUGIN_SCRIPTED_STATE_NONE; + + // Only need to call locationChange if the PopupNotifications object + // for this window has already been initialized (i.e. its getter no + // longer exists) + if (!Object.getOwnPropertyDescriptor(window, "PopupNotifications").get) + PopupNotifications.locationChange(aBrowser); } FullZoom.onLocationChange(aLocationURI, false, aBrowser); } diff --git a/browser/base/content/test/browser_popupNotification.js b/browser/base/content/test/browser_popupNotification.js index df2284b50c02..2fe8fa06bc28 100644 --- a/browser/base/content/test/browser_popupNotification.js +++ b/browser/base/content/test/browser_popupNotification.js @@ -27,16 +27,16 @@ var gActiveListeners = {}; var gActiveObservers = {}; var gShownState = {}; +function goNext() { + if (++gTestIndex == tests.length) + executeSoon(finish); + else + executeSoon(runNextTest); +} + function runNextTest() { let nextTest = tests[gTestIndex]; - function goNext() { - if (++gTestIndex == tests.length) - executeSoon(finish); - else - executeSoon(runNextTest); - } - function addObserver(topic) { function observer() { Services.obs.removeObserver(observer, "PopupNotifications-" + topic); @@ -54,7 +54,7 @@ function runNextTest() { addObserver("backgroundShow"); } else if (nextTest.updateNotShowing) { addObserver("updateNotShowing"); - } else { + } else if (nextTest.onShown) { doOnPopupEvent("popupshowing", function () { info("[Test #" + gTestIndex + "] popup showing"); }); @@ -724,6 +724,34 @@ var tests = [ ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback was not triggered"); PopupNotifications.buttonDelay = PREF_SECURITY_DELAY_INITIAL; }, + }, + { // Test #25 - location change in background tab removes notification + run: function () { + this.oldSelectedTab = gBrowser.selectedTab; + this.newTab = gBrowser.addTab("about:blank"); + gBrowser.selectedTab = this.newTab; + + loadURI("http://example.com/", function() { + gBrowser.selectedTab = this.oldSelectedTab; + let browser = gBrowser.getBrowserForTab(this.newTab); + + this.notifyObj = new basicNotification(); + this.notifyObj.browser = browser; + this.notifyObj.options.eventCallback = function (eventName) { + if (eventName == "removed") { + ok(true, "Notification removed in background tab after reloading"); + executeSoon(function () { + gBrowser.removeTab(this.newTab); + goNext(); + }.bind(this)); + } + }.bind(this); + this.notification = showNotification(this.notifyObj); + executeSoon(function () { + browser.reload(); + }); + }.bind(this)); + } } ]; diff --git a/toolkit/content/PopupNotifications.jsm b/toolkit/content/PopupNotifications.jsm index 6236d939caf0..bf7710f9b600 100644 --- a/toolkit/content/PopupNotifications.jsm +++ b/toolkit/content/PopupNotifications.jsm @@ -277,11 +277,16 @@ PopupNotifications.prototype = { }, /** - * Called by the consumer to indicate that the current browser's location has - * changed, so that we can update the active notifications accordingly. + * Called by the consumer to indicate that a browser's location has changed, + * so that we can update the active notifications accordingly. */ - locationChange: function PopupNotifications_locationChange() { - this._currentNotifications = this._currentNotifications.filter(function(notification) { + locationChange: function PopupNotifications_locationChange(aBrowser) { + if (!aBrowser) + throw "PopupNotifications_locationChange: invalid browser"; + + let notifications = this._getNotificationsForBrowser(aBrowser); + + notifications = notifications.filter(function (notification) { // The persistWhileVisible option allows an open notification to persist // across location changes if (notification.options.persistWhileVisible && @@ -310,7 +315,10 @@ PopupNotifications.prototype = { return false; }, this); - this._update(); + this._setNotificationsForBrowser(aBrowser, notifications); + + if (aBrowser == this.tabbrowser.selectedBrowser) + this._update(); }, /** @@ -356,14 +364,11 @@ PopupNotifications.prototype = { _currentAnchorElement: null, /** - * Gets and sets notifications for the currently selected browser. + * Gets notifications for the currently selected browser. */ get _currentNotifications() { return this._getNotificationsForBrowser(this.tabbrowser.selectedBrowser); }, - set _currentNotifications(a) { - return this._setNotificationsForBrowser(this.tabbrowser.selectedBrowser, a); - }, _remove: function PopupNotifications_removeHelper(notification) { // This notification may already be removed, in which case let's just fail