Only show new notifications in the screen shield

Rework the count system in Source, to distinguish between the
normal notification count and the count of unseen/unacknowledged
notification. (A notification is considered unacknowledged until
shown, as a banner or inside the summary box pointer).
Includes some code cleanups and a test for multiple notifications
in the same source.

https://bugzilla.gnome.org/show_bug.cgi?id=681143

js/ui/messageTray.js

@@ -421,6 +421,7 @@ const Notification = new Lang.Class({
         this.isTransient = false;
         this.expanded = false;
         this.showWhenLocked = false;
+        this.acknowledged = false;
         this._destroyed = false;
         this._useActionIcons = false;
         this._customContent = false;
@@ -1062,7 +1063,7 @@ const SourceActor = new Lang.Class({
         this.actor.add_actor(this._iconBin);
         this.actor.add_actor(this._counterBin);
 
-        this._source.connect('count-changed', Lang.bind(this, this._updateCount));
+        this._source.connect('count-updated', Lang.bind(this, this._updateCount));
         this._updateCount();
     },
 
@@ -1124,8 +1125,6 @@ const Source = new Lang.Class({
         this.iconName = iconName;
         this.iconType = iconType;
 
-        this.count = 0;
-
         this.isTransient = false;
         this.isChat = false;
         this.isMuted = false;
@@ -1136,18 +1135,20 @@ const Source = new Lang.Class({
         this._setSummaryIcon(this.createIcon(this.ICON_SIZE));
     },
 
-    _setCount: function(count, visible) {
+    get count() {
-        if (isNaN(parseInt(count)))
+        return this.notifications.length;
-            throw new Error("Invalid notification count: " + count);
-
-        this.count = count;
-        this.countVisible = visible;
-        this.emit('count-changed');
     },
 
-    _updateCount: function() {
+    get unseenCount() {
-        let count = this.notifications.length;
+        return this.notifications.filter(function(n) { return !n.acknowledged; }).length;
-        this._setCount(count, count > 1);
+    },
+
+    get countVisible() {
+        return this.count > 1;
+    },
+
+    countUpdated: function() {
+        this.emit('count-updated');
     },
 
     setTransient: function(isTransient) {
@@ -1198,13 +1199,14 @@ const Source = new Lang.Class({
                 if (this.notifications.length == 0)
                     this._lastNotificationRemoved();
 
-                this._updateCount();
+                this.countUpdated();
             }));
 
-        this._updateCount();
+        this.countUpdated();
     },
 
     notify: function(notification) {
+        notification.acknowledged = false;
         this.pushNotification(notification);
         if (!this.isMuted)
              this.emit('notify', notification);
@@ -1236,7 +1238,7 @@ const Source = new Lang.Class({
             if (!this.notifications[i].resident)
                 this.notifications[i].destroy();
 
-        this._updateCount();
+        this.countUpdated();
     },
 
     // Default implementation is to destroy this source, but subclasses can override
@@ -1807,7 +1809,7 @@ const MessageTray = new Lang.Class({
 
     _onNotify: function(source, notification) {
         if (this._summaryBoxPointerItem && this._summaryBoxPointerItem.source == source) {
-            if (this._summaryBoxPointerState == State.HIDING)
+            if (this._summaryBoxPointerState == State.HIDING) {
                 // We are in the process of hiding the summary box pointer.
                 // If there is an update for one of the notifications or
                 // a new notification to be added to the notification stack
@@ -1818,6 +1820,14 @@ const MessageTray = new Lang.Class({
                 // need to be able to re-parent its actor to a different
                 // part of the stage.
                 this._reNotifyAfterHideNotification = notification;
+            } else {
+                // The summary box pointer is showing or shown (otherwise,
+                // this._summaryBoxPointerItem would be null)
+                // Immediately mark the notification as acknowledged, as it's
+                // not going into the queue
+                notification.acknowledged = true;
+            }
+
             return;
         }
 
@@ -2309,6 +2319,8 @@ const MessageTray = new Lang.Class({
     },
 
     _updateShowingNotification: function() {
+        this._notification.acknowledged = true;
+
         Tweener.removeTweens(this._notificationBin);
 
         // We auto-expand notifications with CRITICAL urgency.
@@ -2511,10 +2523,17 @@ const MessageTray = new Lang.Class({
         this._summaryBoxPointerDoneDisplayingId = this._summaryBoxPointerItem.connect('done-displaying-content',
                                                                                       Lang.bind(this, this._escapeTray));
         if (this._clickedSummaryItemMouseButton == 1) {
-            this._notificationQueue = this._notificationQueue.filter( Lang.bind(this,
+            let newQueue = [];
-                function(notification) {
+            for (let i = 0; i < this._notificationQueue.length; i++) {
-                    return this._summaryBoxPointerItem.source != notification.source;
+                let notification = this._notificationQueue[i];
-                }));
+                let sameSource = this._summaryBoxPointerItem.source == notification.source;
+                if (sameSource)
+                    notification.acknowledged = true;
+                else
+                    newQueue.push(notification);
+            }
+            this._notificationQueue = newQueue;
+
             this._summaryBoxPointerItem.prepareNotificationStackForShowing();
             this._summaryBoxPointer.bin.child = this._summaryBoxPointerItem.notificationStackView;
             this._summaryBoxPointerItem.scrollTo(St.Side.BOTTOM);

js/ui/screenShield.js

@@ -120,11 +120,11 @@ const NotificationsBox = new Lang.Class({
         return source.hasResidentNotification() && !source.isChat;
     },
 
-    _makeNotificationCountText: function(source) {
+    _makeNotificationCountText: function(count, isChat) {
-        if (source.isChat)
+        if (isChat)
-            return ngettext("%d new message", "%d new messages", source.count).format(source.count);
+            return ngettext("%d new message", "%d new messages", count).format(count);
         else
-            return ngettext("%d new notification", "%d new notifications", source.count).format(source.count);
+            return ngettext("%d new notification", "%d new notifications", count).format(count);
     },
 
     _makeNotificationSource: function(source) {
@@ -142,11 +142,12 @@ const NotificationsBox = new Lang.Class({
                                    style_class: 'screen-shield-notification-label' });
         textBox.add(label);
 
-        let countLabel = new St.Label({ text: this._makeNotificationCountText(source),
+        let count = source.unseenCount;
+        let countLabel = new St.Label({ text: this._makeNotificationCountText(count, source.isChat),
                                         style_class: 'screen-shield-notification-count-text' });
         textBox.add(countLabel);
 
-        box.visible = source.count != 0;
+        box.visible = count != 0;
         return [box, countLabel];
     },
 
@@ -174,7 +175,7 @@ const NotificationsBox = new Lang.Class({
         }
 
         obj.contentUpdatedId = item.connect('content-updated', Lang.bind(this, this._onItemContentUpdated));
-        obj.sourceCountChangedId = item.source.connect('count-changed', Lang.bind(this, this._onSourceChanged));
+        obj.sourceCountChangedId = item.source.connect('count-updated', Lang.bind(this, this._onSourceChanged));
         obj.sourceTitleChangedId = item.source.connect('title-changed', Lang.bind(this, this._onSourceChanged));
         obj.sourceDestroyId = item.source.connect('destroy', Lang.bind(this, this._onSourceDestroy));
         this._items.push(obj);
@@ -225,8 +226,9 @@ const NotificationsBox = new Lang.Class({
             this._residentNotificationBox.add(obj.item.notificationStackView);
         } else {
             // just update the counter
-            obj.countLabel.text = this._makeNotificationCountText(obj.item.source);
+            let count = obj.source.unseenCount;
-            obj.sourceBox.visible = obj.source.count != 0;
+            obj.countLabel.text = this._makeNotificationCountText(count, obj.source.isChat);
+            obj.sourceBox.visible = count != 0;
         }
 
         this._updateVisibility();

js/ui/telepathyClient.js

@@ -578,7 +578,7 @@ const ChatSource = new Lang.Class({
             this._pendingMessages.push(message);
         }
 
-        this._updateCount();
+        this.countUpdated();
 
         let showTimestamp = false;
 
@@ -624,8 +624,17 @@ const ChatSource = new Lang.Class({
         this.destroy();
     },
 
-    _updateCount: function() {
+    /* All messages are new messages for Telepathy sources */
-        this._setCount(this._pendingMessages.length, this._pendingMessages.length > 0);
+    get count() {
+        return this._pendingMessages.length;
+    },
+
+    get unseenCount() {
+        return this.count;
+    },
+
+    get countVisible() {
+        return this.count > 0;
     },
 
     _messageReceived: function(channel, message) {
@@ -633,7 +642,7 @@ const ChatSource = new Lang.Class({
             return;
 
         this._pendingMessages.push(message);
-        this._updateCount();
+        this.countUpdated();
 
         message = makeMessageFromTpMessage(message, NotificationDirection.RECEIVED);
         this._notification.appendMessage(message);
@@ -710,7 +719,7 @@ const ChatSource = new Lang.Class({
 
         if (idx >= 0) {
             this._pendingMessages.splice(idx, 1);
-            this._updateCount();
+            this.countUpdated();
         }
         else
             throw new Error('Message not in our pending list: ' + message);