Inconsistent unread messages count updating #873 (#874)

* Consistent unread messages count updating on ChatBoxView, Minimized ChatBoxView, RosterView and page's title

* Add tests for checking unread messages count updates in different GUI parts

* Update docs/CHANGES.md

* document windowStateChanged event in events.rst

docs/CHANGES.md

@@ -11,6 +11,7 @@
   `chatbox` attributes, instead of just the stanza. [jcbrand]
 - #567 Unreaded message count reset on page load [novokrest]
 - #591 Unread message counter is reset when the chatbox is closed [novokrest]
+- #873 Inconsistent unread messages count updating [novokrest]
 - Remove all inline CSS to comply with strict Content-Security-Policy headers [mathiasertl]
 
 ## 3.0.2 (2017-04-23)

docs/source/events.rst

@@ -283,3 +283,10 @@ serviceDiscovered
 When converse.js has learned of a service provided by the XMPP server. See XEP-0030.
 
 ``_converse.on('serviceDiscovered', function (service) { ... });``
+
+windowStateChanged
+~~~~~~~~~~~~~~~~~~
+
+When window state has changed. Used to determine when a user left the page and when came back.
+
+``_converse.on('windowStateChanged', function (data) { ... });``
\ No newline at end of file

src/converse-chatview.js

@@ -86,6 +86,15 @@
                 },
             });
 
+            var onWindowStateChanged = function (data) {
+                var state = data.state;
+                _converse.chatboxviews.each(function (chatboxview) {
+                    chatboxview.onWindowStateChanged(state);
+                })
+            };
+
+            _converse.api.listen.on('windowStateChanged', onWindowStateChanged);
+
             _converse.ChatBoxView = Backbone.View.extend({
                 length: 200,
                 tagName: 'div',
@@ -422,12 +431,16 @@
                         if (this.model.get('scrolled', true)) {
                             this.$el.find('.new-msgs-indicator').removeClass('hidden');
                         }
-                        if (_converse.windowState === 'hidden' || this.model.get('scrolled', true)) {
-                            _converse.incrementMsgCounter();
+                        if (this.isNewMessageHidden()) {
+                            this.model.incrementUnreadMsgCounter();
                         }
                     }
                 },
 
+                isNewMessageHidden: function() {
+                    return _converse.windowState === 'hidden' || this.model.isScrolledUp();
+                },
+
                 handleTextMessage: function (message) {
                     this.showMessage(_.clone(message.attributes));
                     if (message.get('sender') !== 'me') {
@@ -844,8 +857,8 @@
                         (this.$content.scrollTop() + this.$content.innerHeight()) >=
                             this.$content[0].scrollHeight-10;
                     if (is_at_bottom) {
-                        this.hideNewMessagesIndicator();
                         this.model.save('scrolled', false);
+                        this.onScrolledDown();
                     } else {
                         // We're not at the bottom of the chat area, so we mark
                         // that the box is in a scrolled-up state.
@@ -862,11 +875,19 @@
                     /* Inner method that gets debounced */
                     if (this.$content.is(':visible') && !this.model.get('scrolled')) {
                         this.$content.scrollTop(this.$content[0].scrollHeight);
-                        this.hideNewMessagesIndicator();
+                        this.onScrolledDown();
                         this.model.save({'auto_scrolled': true});
                     }
                 },
 
+                onScrolledDown: function() {
+                    this.hideNewMessagesIndicator();
+                    if (_converse.windowState !== 'hidden') {
+                        this.model.clearUnreadMsgCounter();
+                    }
+                    _converse.emit('chatBoxScrolledDown', {'chatbox': this.model});
+                },
+
                 scrollDown: function () {
                     if (_.isUndefined(this.debouncedScrollDown)) {
                         /* We wrap the method in a debouncer and set it on the
@@ -877,6 +898,12 @@
                     }
                     this.debouncedScrollDown.apply(this, arguments);
                     return this;
+                },
+
+                onWindowStateChanged: function (state) {
+                    if (this.model.get('num_unread', 0) && !this.isNewMessageHidden()) {
+                        this.model.clearUnreadMsgCounter();
+                    }
                 }
             });
         }

src/converse-core.js

@@ -519,26 +519,21 @@
             }
         };
 
-        this.updateMsgCounter = function () {
-            if (this.msg_counter > 0) {
-                if (document.title.search(/^Messages \(\d+\) /) === -1) {
-                    document.title = "Messages (" + this.msg_counter + ") " + document.title;
-                } else {
-                    document.title = document.title.replace(/^Messages \(\d+\) /, "Messages (" + this.msg_counter + ") ");
-                }
-            } else if (document.title.search(/^Messages \(\d+\) /) !== -1) {
-                document.title = document.title.replace(/^Messages \(\d+\) /, "");
-            }
-        };
-
         this.incrementMsgCounter = function () {
             this.msg_counter += 1;
-            this.updateMsgCounter();
+            var unreadMsgCount = this.msg_counter;
+            if (document.title.search(/^Messages \(\d+\) /) === -1) {
+                document.title = "Messages (" + unreadMsgCount + ") " + document.title;
+            } else {
+                document.title = document.title.replace(/^Messages \(\d+\) /, "Messages (" + unreadMsgCount + ") ");
+            }
         };
 
         this.clearMsgCounter = function () {
             this.msg_counter = 0;
-            this.updateMsgCounter();
+            if (document.title.search(/^Messages \(\d+\) /) !== -1) {
+                document.title = document.title.replace(/^Messages \(\d+\) /, "");
+            }
         };
 
         this.initStatus = function () {
@@ -605,6 +600,7 @@
                 _converse.clearMsgCounter();
             }
             _converse.windowState = state;
+            _converse.emit('windowStateChanged', {state: state})
         };
 
         this.registerGlobalEventHandlers = function () {
@@ -1415,6 +1411,19 @@
 
             createMessage: function (message, delay, original_stanza) {
                 return this.messages.create(this.getMessageAttributes.apply(this, arguments));
+            },
+
+            incrementUnreadMsgCounter: function() {
+                this.save({'num_unread': this.get('num_unread') + 1});
+                _converse.incrementMsgCounter();
+            },
+
+            clearUnreadMsgCounter: function() {
+                this.save({'num_unread': 0});
+            },
+
+            isScrolledUp: function () {
+                return this.get('scrolled', true);
             }
         });
 

src/converse-minimize.js

@@ -104,6 +104,11 @@
                     }
                 },
 
+                isNewMessageHidden: function () {
+                    return this.model.get('minimized') ||
+                        this.__super__.isNewMessageHidden.apply(this, arguments);
+                },
+
                 shouldShowOnTextMessage: function () {
                     return !this.model.get('minimized') &&
                         this.__super__.shouldShowOnTextMessage.apply(this, arguments);
@@ -133,6 +138,9 @@
                     // Restores a minimized chat box
                     var _converse = this.__super__._converse;
                     this.$el.insertAfter(_converse.chatboxviews.get("controlbox").$el);
+                    if (!this.model.isScrolledUp()) {
+                        this.model.clearUnreadMsgCounter();
+                    }
                     this.show();
                     _converse.emit('chatBoxMaximized', this);
                     return this;
@@ -312,15 +320,7 @@
                 },
 
                 initialize: function () {
-                    this.model.messages.on('add', function (m) {
-                        if (m.get('message')) {
-                            this.updateUnreadMessagesCounter();
-                        }
-                    }, this);
-                    this.model.on('change:minimized', this.clearUnreadMessagesCounter, this);
-                    // OTR stuff, doesn't require this module to depend on OTR.
-                    this.model.on('showReceivedOTRMessage', this.updateUnreadMessagesCounter, this);
-                    this.model.on('showSentOTRMessage', this.updateUnreadMessagesCounter, this);
+                    this.model.on('change:num_unread', this.render, this);
                 },
 
                 render: function () {
@@ -338,16 +338,6 @@
                     return this.$el.html(tpl_trimmed_chat(data));
                 },
 
-                clearUnreadMessagesCounter: function () {
-                    this.model.save({'num_unread': 0});
-                    this.render();
-                },
-
-                updateUnreadMessagesCounter: function () {
-                    this.model.save({'num_unread': this.model.get('num_unread') + 1});
-                    this.render();
-                },
-
                 close: function (ev) {
                     if (ev && ev.preventDefault) { ev.preventDefault(); }
                     this.remove();
@@ -365,7 +355,7 @@
 
                 restore: _.debounce(function (ev) {
                     if (ev && ev.preventDefault) { ev.preventDefault(); }
-                    this.model.messages.off('add',null,this);
+                    this.model.off('change:num_unread', null, this);
                     this.remove();
                     this.model.maximize();
                 }, 200, {'leading': true})

src/converse-rosterview.js

@@ -689,7 +689,6 @@
 
                 openChat: function (ev) {
                     if (ev && ev.preventDefault) { ev.preventDefault(); }
-                    this.model.save({'num_unread': 0});
                     return _converse.chatboxviews.showChat(this.model.attributes, true);
                 },
 
@@ -935,14 +934,14 @@
 
             var onChatBoxMaximized = function (chatboxview) {
                 /* When a chat box gets maximized, the num_unread counter needs
-                 * to be cleared.
+                 * to be cleared, but if chatbox is scrolled up, then num_unread should not be cleared.
                  */
                 var chatbox = chatboxview.model;
                 if (chatbox.get('type') === 'chatroom') {
                     // TODO
                 } else {
                     var contact = _.head(_converse.roster.where({'jid': chatbox.get('jid')}));
-                    if (!_.isUndefined(contact)) {
+                    if (!_.isUndefined(contact) && !chatbox.isScrolledUp()) {
                         contact.save({'num_unread': 0});
                     }
                 }
@@ -960,9 +959,9 @@
                     return; // The message has no text
                 }
                 var new_message = !(sizzle('result[xmlns="'+Strophe.NS.MAM+'"]', data.stanza).length);
-                var hidden_or_minimized_chatbox = chatbox.get('hidden') || chatbox.get('minimized');
+                var is_new_message_hidden = chatbox.get('hidden') || chatbox.get('minimized') || chatbox.isScrolledUp();
 
-                if (hidden_or_minimized_chatbox && new_message) {
+                if (is_new_message_hidden && new_message) {
                     if (chatbox.get('type') === 'chatroom') {
                         // TODO
                     } else {
@@ -974,6 +973,17 @@
                 }
             };
 
+            var onChatBoxScrolledDown = function (data) {
+                var chatbox = data.chatbox;
+                if (_.isUndefined(chatbox)) {
+                    return;
+                }
+                var contact = _.head(_converse.roster.where({'jid': chatbox.get('jid')}));
+                if (!_.isUndefined(contact)) {
+                    contact.save({'num_unread': 0});
+                }
+            };
+
             var initRoster = function () {
                 /* Create an instance of RosterView once the RosterGroups
                  * collection has been created (in converse-core.js)
@@ -987,6 +997,7 @@
             _converse.api.listen.on('rosterReadyAfterReconnection', initRoster);
             _converse.api.listen.on('message', onMessageReceived);
             _converse.api.listen.on('chatBoxMaximized', onChatBoxMaximized);
+            _converse.api.listen.on('chatBoxScrolledDown', onChatBoxScrolledDown);
         }
     });
 }));

tests/utils.js

@@ -3,6 +3,7 @@
 }(this, function (converse_api, Promise, mock, waitUntilPromise) {
     var _ = converse_api.env._;
     var $ = converse_api.env.jQuery;
+    var $msg = converse_api.env.$msg;
     var $pres = converse_api.env.$pres;
     var $iq = converse_api.env.$iq;
     var Strophe = converse_api.env.Strophe;
@@ -216,6 +217,17 @@
         }, converse));
     };
 
+    utils.createChatMessage = function (_converse, sender_jid, message) {
+        return $msg({
+                   from: sender_jid,
+                   to: _converse.connection.jid,
+                   type: 'chat',
+                   id: (new Date()).getTime()
+               })
+               .c('body').t(message).up()
+               .c('active', {'xmlns': Strophe.NS.CHATSTATES}).tree();
+    }
+
     utils.sendMessage = function (chatboxview, message) {
         chatboxview.$el.find('.chat-textarea').val(message);
         chatboxview.$el.find('textarea.chat-textarea').trigger($.Event('keypress', {keyCode: 13}));