Bug 839445 - PopupNotifications need to know about location changes in background tabs. r=gavin
authorDão Gottwald <dao@mozilla.com>
Mon, 18 Feb 2013 22:06:03 +0100
changeset 132131 fc380df34bd6a0055483a554695cf8276d803ca5
parent 132130 69cc1bc86785012023ae4e5870f9c6da162271e5
child 132132 adf96501de917cf1516beaa2ef79c19ae3a4c67a
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgavin
bugs839445
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 839445 - PopupNotifications need to know about location changes in background tabs. r=gavin
browser/base/content/browser.js
browser/base/content/test/browser_popupNotification.js
toolkit/content/PopupNotifications.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4336,28 +4336,16 @@ var XULBrowserWindow = {
       if (gURLBar) {
         URLBarSetURI(aLocationURI);
 
         // Update starring UI
         PlacesStarButton.updateState();
         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");
       } else {
         let ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore);
         if (ss.getTabValue(gBrowser.selectedTab, "appOrigin"))
           document.documentElement.setAttribute("disablechrome", "true");
         else
@@ -4771,16 +4759,22 @@ var TabsProgressListener = {
     if (aBrowser.contentWindow == aWebProgress.DOMWindow) {
       // Filter out any onLocationChanges triggered by anchor navigation
       // or history.push/pop/replaceState.
       if (aRequest) {
         // Initialize the click-to-play state.
         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);
     }
   },
 
   onRefreshAttempted: function (aBrowser, aWebProgress, aURI, aDelay, aSameURI) {
     if (gPrefService.getBoolPref("accessibility.blockautorefresh")) {
       let brandBundle = document.getElementById("bundle_brand");
--- a/browser/base/content/test/browser_popupNotification.js
+++ b/browser/base/content/test/browser_popupNotification.js
@@ -22,26 +22,26 @@ function cleanUp() {
 }
 
 const PREF_SECURITY_DELAY_INITIAL = Services.prefs.getIntPref("security.notification_enable_delay");
 
 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);
       delete gActiveObservers["PopupNotifications-" + topic];
 
       info("[Test #" + gTestIndex + "] observer for " + topic + " called");
       nextTest[topic]();
       goNext();
@@ -49,17 +49,17 @@ function runNextTest() {
     Services.obs.addObserver(observer, "PopupNotifications-" + topic, false);
     gActiveObservers["PopupNotifications-" + topic] = observer;
   }
 
   if (nextTest.backgroundShow) {
     addObserver("backgroundShow");
   } else if (nextTest.updateNotShowing) {
     addObserver("updateNotShowing");
-  } else {
+  } else if (nextTest.onShown) {
     doOnPopupEvent("popupshowing", function () {
       info("[Test #" + gTestIndex + "] popup showing");
     });
     doOnPopupEvent("popupshown", function () {
       gShownState[gTestIndex] = true;
       info("[Test #" + gTestIndex + "] popup shown");
       nextTest.onShown(this);
     });
@@ -719,16 +719,44 @@ var tests = [
       }, 500);
 
     },
     onHidden: function (popup) {
       ok(this.notifyObj.mainActionClicked, "mainAction was clicked after the delay");
       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));
+    }
   }
 ];
 
 function showNotification(notifyObj) {
   return PopupNotifications.show(notifyObj.browser,
                                  notifyObj.id,
                                  notifyObj.message,
                                  notifyObj.anchorID,
--- a/toolkit/content/PopupNotifications.jsm
+++ b/toolkit/content/PopupNotifications.jsm
@@ -272,21 +272,26 @@ PopupNotifications.prototype = {
    */
   get isPanelOpen() {
     let panelState = this.panel.state;
 
     return panelState == "showing" || panelState == "open";
   },
 
   /**
-   * 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 &&
           this.isPanelOpen) {
         if ("persistence" in notification.options &&
           notification.options.persistence)
           notification.options.persistence--;
         return true;
@@ -305,17 +310,20 @@ PopupNotifications.prototype = {
           Date.now() <= notification.options.timeout) {
         return true;
       }
 
       this._fireCallback(notification, NOTIFICATION_EVENT_REMOVED);
       return false;
     }, this);
 
-    this._update();
+    this._setNotificationsForBrowser(aBrowser, notifications);
+
+    if (aBrowser == this.tabbrowser.selectedBrowser)
+      this._update();
   },
 
   /**
    * Removes a Notification.
    * @param notification
    *        The Notification object to remove.
    */
   remove: function PopupNotifications_remove(notification) {
@@ -351,24 +359,21 @@ PopupNotifications.prototype = {
 ////////////////////////////////////////////////////////////////////////////////
 // Utility methods
 ////////////////////////////////////////////////////////////////////////////////
 
   _ignoreDismissal: null,
   _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
     // silently.
     let notifications = this._getNotificationsForBrowser(notification.browser);
     if (!notifications)
       return;