Bug 457632: notificationbox doesn't remove notifications immediately. r=neil@parkwaycc.co.uk
authorDave Townsend <dtownsend@oxymoronical.com>
Thu, 09 Oct 2008 10:35:01 +0100
changeset 20196 b55e95f2997345fc0eba8c7a05130f0190ebf0c5
parent 20195 a19ab72868025003549c496b6292a27184d4cf0b
child 20197 62274f98ccf834fef92e40f9b6afc9e235ab3596
push idunknown
push userunknown
push dateunknown
reviewersneil
bugs457632
milestone1.9.1b2pre
Bug 457632: notificationbox doesn't remove notifications immediately. r=neil@parkwaycc.co.uk
toolkit/content/tests/widgets/Makefile.in
toolkit/content/tests/widgets/test_bug457632.xul
toolkit/content/widgets/notification.xml
--- a/toolkit/content/tests/widgets/Makefile.in
+++ b/toolkit/content/tests/widgets/Makefile.in
@@ -43,16 +43,17 @@ relativesrcdir  = toolkit/content/tests/
 
 include $(DEPTH)/config/autoconf.mk
 include $(topsrcdir)/config/rules.mk
 
 _TEST_FILES = 	test_bug360220.xul \
 		test_bug359754.xul \
 		test_bug365773.xul \
 		test_bug382990.xul \
+		test_bug457632.xul \
 		test_closemenu_attribute.xul \
 		test_colorpicker_popup.xul \
 		test_deck.xul \
 		test_menulist_keynav.xul \
 		test_menulist_null_value.xul \
 		test_popup_coords.xul \
 		test_popup_recreate.xul \
 		test_popup_button.xul \
new file mode 100644
--- /dev/null
+++ b/toolkit/content/tests/widgets/test_bug457632.xul
@@ -0,0 +1,179 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="/tests/SimpleTest/test.css" type="text/css"?>
+<!--
+  XUL Widget Test for bug 457632
+  -->
+<window title="Bug 457632" width="500" height="600"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>  
+
+  <notificationbox id="nb"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml" style="height: 300px; overflow: auto;"
+        onload="test()"/>
+
+  <!-- test code goes here -->
+<script type="application/javascript">
+<![CDATA[
+var gNotificationBox;
+
+function completeAnimation(nextTest) {
+  if (!gNotificationBox._timer) {
+    nextTest();
+    return;
+  }
+
+  setTimeout(completeAnimation, 50, nextTest);
+}
+
+function test() {
+  SimpleTest.waitForExplicitFinish();
+  gNotificationBox = document.getElementById("nb");
+
+  is(gNotificationBox.allNotifications.length, 0, "There should be no initial notifications");
+  gNotificationBox.appendNotification("Test notification",
+                                      "notification1", null,
+                                      gNotificationBox.PRIORITY_INFO_LOW,
+                                      null);
+  is(gNotificationBox.allNotifications.length, 1, "Notification exists while animating in");
+  let notification = gNotificationBox.getNotificationWithValue("notification1");
+  ok(notification, "Notification should exist while animating in");
+
+  // Wait for the notificaton to finish displaying
+  completeAnimation(test1);
+}
+
+// Tests that a notification that is fully animated in gets removed immediately
+function test1() {
+  let notification = gNotificationBox.getNotificationWithValue("notification1");
+  gNotificationBox.removeNotification(notification);
+  notification = gNotificationBox.getNotificationWithValue("notification1");
+  ok(!notification, "Test 1 showed notification was still present");
+  ok(!gNotificationBox.currentNotification, "Test 1 said there was still a current notification");
+  is(gNotificationBox.allNotifications.length, 0, "Test 1 should show no notifications present");
+
+  // Wait for the notificaton to finish hiding
+  completeAnimation(test2);
+}
+
+// Tests that a notification that is animating in gets removed immediately
+function test2() {
+  let notification = gNotificationBox.appendNotification("Test notification",
+                                                         "notification2", null,
+                                                         gNotificationBox.PRIORITY_INFO_LOW,
+                                                         null);
+  gNotificationBox.removeNotification(notification);
+  notification = gNotificationBox.getNotificationWithValue("notification2");
+  ok(!notification, "Test 2 showed notification was still present");
+  ok(!gNotificationBox.currentNotification, "Test 2 said there was still a current notification");
+  is(gNotificationBox.allNotifications.length, 0, "Test 2 should show no notifications present");
+
+  // Get rid of the hiding notifications
+  gNotificationBox.removeAllNotifications(true);
+  test3();
+}
+
+// Tests that a background notification goes away immediately
+function test3() {
+  let notification = gNotificationBox.appendNotification("Test notification",
+                                                         "notification3", null,
+                                                         gNotificationBox.PRIORITY_INFO_LOW,
+                                                         null);
+  let notification2 = gNotificationBox.appendNotification("Test notification",
+                                                          "notification4", null,
+                                                          gNotificationBox.PRIORITY_INFO_LOW,
+                                                          null);
+  is(gNotificationBox.allNotifications.length, 2, "Test 3 should show 2 notifications present");
+  gNotificationBox.removeNotification(notification);
+  is(gNotificationBox.allNotifications.length, 1, "Test 3 should show 1 notifications present");
+  notification = gNotificationBox.getNotificationWithValue("notification3");
+  ok(!notification, "Test 3 showed notification was still present");
+  gNotificationBox.removeNotification(notification2);
+  is(gNotificationBox.allNotifications.length, 0, "Test 3 should show 0 notifications present");
+  notification2 = gNotificationBox.getNotificationWithValue("notification4");
+  ok(!notification2, "Test 3 showed notification2 was still present");
+  ok(!gNotificationBox.currentNotification, "Test 3 said there was still a current notification");
+
+  // Get rid of the hiding notifications
+  gNotificationBox.removeAllNotifications(true);
+  test4();
+}
+
+// Tests that a foreground notification hiding a background one goes away
+function test4() {
+  let notification = gNotificationBox.appendNotification("Test notification",
+                                                         "notification5", null,
+                                                         gNotificationBox.PRIORITY_INFO_LOW,
+                                                         null);
+  let notification2 = gNotificationBox.appendNotification("Test notification",
+                                                          "notification6", null,
+                                                          gNotificationBox.PRIORITY_INFO_LOW,
+                                                          null);
+  gNotificationBox.removeNotification(notification2);
+  notification2 = gNotificationBox.getNotificationWithValue("notification6");
+  ok(!notification2, "Test 4 showed notification2 was still present");
+  is(gNotificationBox.currentNotification, notification, "Test 4 said the current notification was wrong");
+  is(gNotificationBox.allNotifications.length, 1, "Test 4 should show 1 notifications present");
+  gNotificationBox.removeNotification(notification);
+  notification = gNotificationBox.getNotificationWithValue("notification5");
+  ok(!notification, "Test 4 showed notification was still present");
+  ok(!gNotificationBox.currentNotification, "Test 4 said there was still a current notification");
+  is(gNotificationBox.allNotifications.length, 0, "Test 4 should show 0 notifications present");
+
+  // Get rid of the hiding notifications
+  gNotificationBox.removeAllNotifications(true);
+  test5();
+}
+
+// Tests that removeAllNotifications gets rid of everything
+function test5() {
+  let notification = gNotificationBox.appendNotification("Test notification",
+                                                         "notification7", null,
+                                                         gNotificationBox.PRIORITY_INFO_LOW,
+                                                         null);
+  let notification2 = gNotificationBox.appendNotification("Test notification",
+                                                          "notification8", null,
+                                                          gNotificationBox.PRIORITY_INFO_LOW,
+                                                          null);
+  gNotificationBox.removeAllNotifications();
+  notification = gNotificationBox.getNotificationWithValue("notification7");
+  notification2 = gNotificationBox.getNotificationWithValue("notification8");
+  ok(!notification, "Test 5 showed notification was still present");
+  ok(!notification2, "Test 5 showed notification2 was still present");
+  ok(!gNotificationBox.currentNotification, "Test 5 said there was still a current notification");
+  is(gNotificationBox.allNotifications.length, 0, "Test 5 should show 0 notifications present");
+
+  gNotificationBox.appendNotification("Test notification",
+                                      "notification9", null,
+                                      gNotificationBox.PRIORITY_INFO_LOW,
+                                      null);
+
+  // Wait for the notificaton to finish displaying
+  completeAnimation(test6);
+}
+
+// Tests whether removing an already removed notification doesn't break things
+function test6() {
+  let notification = gNotificationBox.getNotificationWithValue("notification9");
+  ok(notification, "Test 6 should have an initial notification");
+  gNotificationBox.removeNotification(notification);
+  gNotificationBox.removeNotification(notification);
+
+  ok(!gNotificationBox.currentNotification, "Test 6 shouldn't be any current notification");
+  is(gNotificationBox.allNotifications.length, 0, "Test 6 allNotifications.length should be 0");
+  notification = gNotificationBox.appendNotification("Test notification",
+                                                     "notification10", null,
+                                                     gNotificationBox.PRIORITY_INFO_LOW,
+                                                     null);
+  is(notification, gNotificationBox.currentNotification, "Test 6 should have made the current notification");
+  gNotificationBox.removeNotification(notification);
+
+  SimpleTest.finish();
+}
+]]>
+</script>
+
+</window>
--- a/toolkit/content/widgets/notification.xml
+++ b/toolkit/content/widgets/notification.xml
@@ -43,18 +43,25 @@
         <setter>
           if (val)
             this.setAttribute('notificationshidden', true);
           else this.removeAttribute('notificationshidden');
           return val;
         </setter>
       </property>
 
-      <property name="allNotifications" readonly="true"
-                onget="return this.getElementsByTagName('notification');"/>
+      <property name="allNotifications" readonly="true">
+        <getter>
+        <![CDATA[
+          var closedNotification = this._closedNotification;
+          var notifications = this.getElementsByTagName('notification');
+          return Array.filter(notifications, function(n) n != closedNotification);
+        ]]>
+        </getter>
+      </property>
 
       <method name="getNotificationWithValue">
         <parameter name="aValue"/>
         <body>
           <![CDATA[
             var notifications = this.allNotifications;
             for (var n = notifications.length - 1; n >= 0; n--) {
               if (aValue == notifications[n].value)
@@ -133,17 +140,17 @@
       </method>
 
       <method name="removeNotification">
         <parameter name="aItem"/>
         <body>
           <![CDATA[
             if (aItem == this.currentNotification)
               this.removeCurrentNotification();
-            else
+            else if (aItem != this._closedNotification)
               this.removeChild(aItem);
             return aItem;
           ]]>
         </body>
       </method>
 
       <method name="removeCurrentNotification">
         <body>
@@ -161,16 +168,25 @@
             var notifications = this.allNotifications;
             for (var n = notifications.length - 1; n >= 0; n--) {
               if (aImmediate)
                 this.removeChild(notifications[n]);
               else
                 this.removeNotification(notifications[n]);
             }
             this.currentNotification = null;
+
+            if (aImmediate && this._timer) {
+              // Must clear up any currently animating notification
+              clearInterval(this._timer);
+              if (this._closedNotification) {
+                this.removeChild(this._closedNotification);
+                this._closedNotification = null;
+              }
+            }
           ]]>
         </body>
       </method>
 
       <method name="removeTransientNotifications">
         <body>
           <![CDATA[
             var notifications = this.allNotifications;
@@ -191,55 +207,62 @@
         <body>
           <![CDATA[
             if (this._timer) {
               clearInterval(this._timer);
               if (this.currentNotification) {
                 this.currentNotification.style.marginTop = "0px";
                 this.currentNotification.style.opacity = 1;
               }
-              if (this._closedNotification)
-                this._closedNotification.parentNode.
-                  removeChild(this._closedNotification);
+              if (this._closedNotification) {
+                this.removeChild(this._closedNotification);
+                this._closedNotification = null;
+              }
               this._setBlockingState(this.currentNotification);
             }
 
             var height = aNotification.boxObject.height;
             var change = height / this.slideSteps;
             var margin;
             if (aSlideIn) {
               if (this.currentNotification &&
                   this.currentNotification.boxObject.height > height)
                 height = this.currentNotification.boxObject.height;
 
               this.currentNotification = aNotification;
-              this._closedNotification = null;
               aNotification.style.removeProperty("position");
               aNotification.style.removeProperty("top");
               aNotification.style.marginTop = -height + "px";
               aNotification.style.opacity = 0;
               margin = -height;
             }
             else {
               change = -change;
               this._closedNotification = aNotification;
               var notifications = this.allNotifications;
-              var idx = notifications.length - 2;
+              var idx = notifications.length - 1;
               if (idx >= 0)
                 this.currentNotification = notifications[idx];
               else
                 this.currentNotification = null;
               var style = window.getComputedStyle(aNotification, null);
               margin = style.getPropertyCSSValue("margin-top").
                          getFloatValue(CSSPrimitiveValue.CSS_PX);
             }
             var opacitychange = change / height;
             const FRAME_LENGTH = 50;
 
-            function slide(self, off) {
+            function slide(self, args, off) {
+              // If the notificationbox is disconnected then stop the timer
+              if (self.ownerDocument.compareDocumentPosition(self) &
+                  Node.DOCUMENT_POSITION_DISCONNECTED) {
+                clearInterval(args.timer);
+                return;
+              }
+
               var framesToHandle = 1;
 
               // Skip frames if we aren't getting the desired frame rate.
               if (off > 0)
                 framesToHandle += Math.round(off / FRAME_LENGTH);
 
               margin += framesToHandle * change;
 
@@ -255,23 +278,26 @@
                 if (opacitychange)
                   aNotification.style.opacity =
                     Number(aNotification.style.opacity) + framesToHandle * opacitychange;
                 return;
               }
 
               clearInterval(self._timer);
               self._timer = null;
-              if (self._closedNotification)
-                self._closedNotification.parentNode.
-                  removeChild(self._closedNotification);
+              if (self._closedNotification) {
+                self.removeChild(self._closedNotification);
+                self._closedNotification = null;
+              }
               self._setBlockingState(self.currentNotification);
             }
 
-            this._timer = setInterval(slide, FRAME_LENGTH, this);
+            var args = {};
+            this._timer = setInterval(slide, FRAME_LENGTH, this, args);
+            args.timer = this._timer;
           ]]>
         </body>
       </method>
 
       <method name="_setBlockingState">
         <parameter name="aNotification"/>
         <body>
           <![CDATA[