From f60ee4d6408d4f593921263de15bf8ffc8951cf2 Mon Sep 17 00:00:00 2001
From: JC Brand <jc@opkode.com>
Date: Thu, 13 Oct 2016 18:22:37 +0200
Subject: [PATCH] Refactor rooms to have separate handlers for presence and
 message stanzas

---
 spec/chatroom.js    | 12 ++-------
 src/converse-muc.js | 64 +++++++++++++++++++++++++++------------------
 2 files changed, 41 insertions(+), 35 deletions(-)

diff --git a/spec/chatroom.js b/spec/chatroom.js
index 42c106644..8c29dac64 100644
--- a/spec/chatroom.js
+++ b/spec/chatroom.js
@@ -173,9 +173,8 @@
             it("shows users currently present in the room", function () {
                 test_utils.openAndEnterChatRoom('lounge', 'localhost', 'dummy');
                 var name;
-                var view = this.chatboxviews.get('lounge@localhost'),
+                var view = converse.chatboxviews.get('lounge@localhost'),
                     $occupants = view.$('.occupant-list');
-                spyOn(view, 'onChatRoomPresence').andCallThrough();
                 var presence, role;
                 for (var i=0; i<mock.chatroom_names.length; i++) {
                     name = mock.chatroom_names[i];
@@ -191,9 +190,7 @@
                         role: role
                     }).up()
                     .c('status').attrs({code:'110'}).nodeTree;
-
                     converse.connection._dataRecv(test_utils.createRequest(presence));
-                    expect(view.onChatRoomPresence).toHaveBeenCalled();
                     expect($occupants.find('li').length).toBe(2+i);
                     expect($($occupants.find('li')[i+1]).text()).toBe(mock.chatroom_names[i]);
                     expect($($occupants.find('li')[i+1]).hasClass('moderator')).toBe(role === "moderator");
@@ -216,10 +213,9 @@
                         role: 'none'
                     }).nodeTree;
                     converse.connection._dataRecv(test_utils.createRequest(presence));
-                    expect(view.onChatRoomPresence).toHaveBeenCalled();
                     expect($occupants.find('li').length).toBe(i+1);
                 }
-            }.bind(converse));
+            });
 
             it("indicates moderators by means of a special css class and tooltip", function () {
                 test_utils.openAndEnterChatRoom('lounge', 'localhost', 'dummy');
@@ -524,7 +520,6 @@
                 test_utils.openAndEnterChatRoom('lounge', 'localhost', 'oldnick');
                 var view = converse.chatboxviews.get('lounge@localhost');
                 var $chat_content = view.$el.find('.chat-content');
-                spyOn(view, 'onChatRoomPresence').andCallThrough();
 
                 // The user has just entered the room and receives their own
                 // presence from the server.
@@ -544,7 +539,6 @@
                   .c('status').attrs({code:'210'}).nodeTree;
 
                 converse.connection._dataRecv(test_utils.createRequest(presence));
-                expect(view.onChatRoomPresence).toHaveBeenCalled();
                 var $occupants = view.$('.occupant-list');
                 expect($occupants.children().length).toBe(1);
                 expect($occupants.children().first(0).text()).toBe("oldnick");
@@ -569,7 +563,6 @@
                     .c('status').attrs({code:'110'}).nodeTree;
 
                 converse.connection._dataRecv(test_utils.createRequest(presence));
-                expect(view.onChatRoomPresence).toHaveBeenCalled();
                 expect($chat_content.find('div.chat-info').length).toBe(2);
                 expect($chat_content.find('div.chat-info').last().html()).toBe(__(view.newNicknameMessages["303"], "newnick"));
 
@@ -590,7 +583,6 @@
                     .c('status').attrs({code:'110'}).nodeTree;
 
                 converse.connection._dataRecv(test_utils.createRequest(presence));
-                expect(view.onChatRoomPresence).toHaveBeenCalled();
                 expect($chat_content.find('div.chat-info').length).toBe(2);
                 expect($chat_content.find('div.chat-info').last().html()).toBe(__(view.newNicknameMessages["303"], "newnick"));
                 $occupants = view.$('.occupant-list');
diff --git a/src/converse-muc.js b/src/converse-muc.js
index e92037590..bbdcd9e9e 100755
--- a/src/converse-muc.js
+++ b/src/converse-muc.js
@@ -306,7 +306,6 @@
                 },
 
                 close: function (ev) {
-                    converse.connection.deleteHandler(this.handler);
                     this.leave();
                     converse.ChatBoxView.prototype.close.apply(this, arguments);
                 },
@@ -586,27 +585,12 @@
                     }
                 },
 
-                handleMUCStanza: function (stanza) {
-                    var xmlns, xquery, i;
-                    var from = stanza.getAttribute('from');
+                handleMUCMessage: function (stanza) {
                     var is_mam = $(stanza).find('[xmlns="'+Strophe.NS.MAM+'"]').length > 0;
-                    if (!from || (this.model.get('id') !== from.split("/")[0])  || is_mam) {
+                    if (is_mam) {
                         return true;
                     }
-                    if (stanza.nodeName === "message") {
-                        _.compose(this.onChatRoomMessage.bind(this), this.showStatusMessages.bind(this))(stanza);
-                    } else if (stanza.nodeName === "presence") {
-                        xquery = stanza.getElementsByTagName("x");
-                        if (xquery.length > 0) {
-                            for (i = 0; i < xquery.length; i++) {
-                                xmlns = xquery[i].getAttribute("xmlns");
-                                if (xmlns && xmlns.match(Strophe.NS.MUC)) {
-                                    this.onChatRoomPresence(stanza);
-                                    break;
-                                }
-                            }
-                        }
-                    }
+                    _.compose(this.onChatRoomMessage.bind(this), this.showStatusMessages.bind(this))(stanza);
                     return true;
                 },
 
@@ -622,7 +606,35 @@
                     return node + "@" + domain + (nick !== null ? "/" + nick : "");
                 },
 
+                registerHandlers: function () {
+                    var room_jid = this.model.get('jid');
+                    this.removeHandlers();
+                    this.presence_handler = converse.connection.addHandler(
+                        this.onChatRoomPresence.bind(this),
+                        Strophe.NS.MUC, 'presence', null, null, room_jid,
+                        {'ignoreNamespaceFragment': true, 'matchBareFromJid': true}
+                    );
+                    this.message_handler = converse.connection.addHandler(
+                        this.handleMUCMessage.bind(this),
+                        null, 'message', null, null, room_jid,
+                        {'matchBareFromJid': true}
+                    );
+                },
+
+                removeHandlers: function () {
+                    if (this.message_handler) {
+                        converse.connection.deleteHandler(this.message_handler);
+                        delete this.message_handler;
+                    }
+                    if (this.presence_handler) {
+                        converse.connection.deleteHandler(this.presence_handler);
+                        delete this.presence_handler;
+                    }
+                    return this;
+                },
+
                 join: function (nick, password) {
+                    this.registerHandlers();
                     var stanza = $pres({
                         'from': converse.connection.jid,
                         'to': this.getRoomJIDAndNick(nick)
@@ -631,9 +643,6 @@
                     if (password) {
                         stanza.cnode(Strophe.xmlElement("password", [], password));
                     }
-                    if (!this.handler) {
-                        this.handler = converse.connection.addHandler(this.handleMUCStanza.bind(this));
-                    }
                     this.model.set('connection_status', Strophe.Status.CONNECTING);
                     return converse.connection.send(stanza);
                 },
@@ -649,11 +658,12 @@
                     if (exit_msg !== null) {
                         presence.c("status", exit_msg);
                     }
+                    var that = this;
                     converse.connection.addHandler(
                         function () {
-                            this.model.set('connection_status', Strophe.Status.DISCONNECTED);
-                        }.bind(this),
-                        null, "presence", null, presenceid);
+                            that.model.set('connection_status', Strophe.Status.DISCONNECTED);
+                            that.removeHandlers();
+                        }, null, "presence", null, presenceid);
                     converse.connection.send(presence);
                 },
 
@@ -1092,6 +1102,7 @@
                         }
                     }
                     this.occupantsview.updateOccupantsOnPresence(pres);
+                    return true;
                 },
 
                 setChatRoomSubject: function (sender, subject) {
@@ -1687,6 +1698,9 @@
                             attrs = {};
                         }
                         var fetcher = converse.chatboxviews.showChat.bind(converse.chatboxviews);
+                        if (!attrs.nick) {
+                            attrs.nick = Strophe.getNodeFromJid(converse.bare_jid);
+                        }
                         if (typeof jids === "undefined") {
                             throw new TypeError('rooms.open: You need to provide at least one JID');
                         } else if (typeof jids === "string") {