Refactor message handling

- use the same method for both normal and OTR messages
- fix /me actions for OTR messages
- rename messaging methods to minimize ambiguity

converse.js

@@ -829,7 +829,7 @@
                 }
             },
 
-            messageReceived: function (message) {
+            receiveMessage: function (message) {
                 var $body = $(message).children('body');
                 var text = ($body.length > 0 ? $body.text() : undefined);
                 if ((!text) || (!converse.allow_otr)) {
@@ -886,10 +886,10 @@
                 this.model.on('showHelpMessages', this.showHelpMessages, this);
                 this.model.on('sendMessageStanza', this.sendMessageStanza, this);
                 this.model.on('showSentOTRMessage', function (text) {
-                    this.showOTRMessage(text, 'me');
+                    this.showMessage({'message': text, 'sender': 'me'});
                 }, this);
                 this.model.on('showReceivedOTRMessage', function (text) {
-                    this.showOTRMessage(text, 'them');
+                    this.showMessage({'message': text, 'sender': 'them'});
                 }, this);
 
                 this.updateVCard();
@@ -937,11 +937,11 @@
                 this.scrollDown();
             },
 
-            showMessage: function ($el, msg_dict) {
-                var this_date = converse.parseISO8601(msg_dict.time),
+            showMessage: function (msg_dict) {
+                var $el = this.$el.find('.chat-content'),
+                    msg_date = msg_dict.time ? converse.parseISO8601(msg_dict.time) : new Date(),
                     text = msg_dict.message,
                     match = text.match(/^\/(.*?)(?: (.*))?$/),
-                    sender = msg_dict.sender,
                     template, username;
 
                 if ((match) && (match[1] === 'me')) {
@@ -950,12 +950,12 @@
                     username = msg_dict.fullname;
                 } else  {
                     template = converse.templates.message;
-                    username = sender === 'me' && __('me') || msg_dict.fullname;
+                    username = msg_dict.sender === 'me' && __('me') || msg_dict.fullname || this.model.get('fullname');
                 }
                 $el.find('div.chat-event').remove();
                 var message = template({
-                    'sender': sender,
-                    'time': this_date.toTimeString().substring(0,5),
+                    'sender': msg_dict.sender,
+                    'time': msg_date.toTimeString().substring(0,5),
                     'username': username,
                     'message': '',
                     'extra_classes': msg_dict.delayed && 'delayed' || ''
@@ -964,24 +964,6 @@
                 return this.scrollDown();
             },
 
-            showOTRMessage:  function (text, sender) {
-                /* "Off-the-record" messages are encrypted and not stored at all,
-                 * so we don't have a backbone converse.Message object to work with.
-                 */
-                var username = sender === 'me' && sender || this.model.get('fullname');
-                var $el = this.$el.find('.chat-content');
-                $el.find('div.chat-event').remove();
-                $el.append(
-                    converse.templates.message({
-                        'sender': sender,
-                        'time': (new Date()).toTimeString().substring(0,5),
-                        'message': text,
-                        'username': username,
-                        'extra_classes': ''
-                    }));
-                return this.scrollDown();
-            },
-
             showHelpMessages: function (msgs, type, spinner) {
                 var $chat_content = this.$el.find('.chat-content'), i,
                     msgs_length = msgs.length;
@@ -1000,7 +982,6 @@
                 var time = message.get('time'),
                     times = this.model.messages.pluck('time'),
                     this_date = converse.parseISO8601(time),
-                    $chat_content = this.$el.find('.chat-content'),
                     previous_message, idx, prev_date, isodate, text, match;
 
                 // If this message is on a different day than the one received
@@ -1013,7 +994,7 @@
                     isodate.setUTCHours(0,0,0,0);
                     isodate = converse.toISOString(isodate);
                     if (this.isDifferentDay(prev_date, this_date)) {
-                        $chat_content.append(converse.templates.new_day({
+                        this.$el.find('.chat-content').append(converse.templates.new_day({
                             isodate: isodate,
                             datestring: this_date.toString().substring(0,15)
                         }));
@@ -1023,7 +1004,7 @@
                     this.showStatusNotification(message.get('fullname')+' '+'is typing');
                     return;
                 } else {
-                    this.showMessage($chat_content, _.clone(message.attributes));
+                    this.showMessage(_.clone(message.attributes));
                 }
                 if ((message.get('sender') != 'me') && (converse.windowState == 'blur')) {
                     converse.incrementMsgCounter();
@@ -1077,9 +1058,9 @@
                             ];
                         this.showHelpMessages(msgs);
                         return;
-                    } else if ((converse.allow_otr) || (match[1] === "endotr")) {
+                    } else if ((converse.allow_otr) && (match[1] === "endotr")) {
                         return this.endOTR();
-                    } else if ((converse.allow_otr) || (match[1] === "otr")) {
+                    } else if ((converse.allow_otr) && (match[1] === "otr")) {
                         return this.model.initiateOTR();
                     }
                 }
@@ -2337,7 +2318,7 @@
                 }
                 if (!body) { return true; }
                 var display_sender = sender === this.model.get('nick') && 'me' || 'room';
-                this.showMessage($chat_content, {
+                this.showMessage({
                     'message': body,
                     'sender': display_sender,
                     'fullname': sender,
@@ -2376,12 +2357,9 @@
             model: converse.ChatBox,
 
             registerMessageHandler: function () {
-                // TODO: Make this method global to converse, trigger an event
-                // and let messageReceived be called via a handler for that
-                // event.
                 converse.connection.addHandler(
                     $.proxy(function (message) {
-                        this.messageReceived(message);
+                        this.onMessage(message);
                         return true;
                     }, this), null, 'message', 'chat');
             },
@@ -2411,7 +2389,7 @@
                 });
             },
 
-            messageReceived: function (message) {
+            onMessage: function (message) {
                 var buddy_jid, $message = $(message),
                     message_from = $message.attr('from');
                 if (message_from == converse.connection.jid) {
@@ -2446,7 +2424,6 @@
                 if (!chatbox) {
                     var fullname = roster_item.get('fullname');
                     fullname = _.isEmpty(fullname)? buddy_jid: fullname;
-
                     chatbox = this.create({
                         'id': buddy_jid,
                         'jid': buddy_jid,
@@ -2456,7 +2433,7 @@
                         'url': roster_item.get('url')
                     });
                 }
-                chatbox.messageReceived(message);
+                chatbox.receiveMessage(message);
                 converse.roster.addResource(buddy_jid, resource);
                 converse.emit('onMessage', message);
                 return true;
@@ -2488,7 +2465,7 @@
                         view.model = item;
                         view.initialize();
                         if (item.get('id') !== 'controlbox') {
-                            // FIXME: Why is it necessary to again append chatboxes?
+                            // XXX: Why is it necessary to again append chatboxes?
                             view.$el.appendTo(this.$el);
                         }
                     }

spec/chatbox.js

@@ -287,6 +287,15 @@
             }, converse));
 
             describe("A Chat Message", $.proxy(function () {
+
+                beforeEach(function () {
+                    runs(function () {
+                        utils.closeAllChatBoxes();
+                    });
+                    waits(250);
+                    runs(function () {});
+                });
+
                 it("can be received which will open a chatbox and be displayed inside it", $.proxy(function () {
                     spyOn(converse, 'emit');
                     var message = 'This is a received message';
@@ -303,9 +312,8 @@
                     expect(this.chatboxes.get(sender_jid)).not.toBeDefined();
 
                     runs($.proxy(function () {
-                        // messageReceived is a handler for received XMPP
-                        // messages
-                        this.chatboxes.messageReceived(msg);
+                        // onMessage is a handler for received XMPP messages
+                        this.chatboxes.onMessage(msg);
                         expect(converse.emit).toHaveBeenCalledWith('onMessage', msg);
                     }, converse));
                     waits(300);
@@ -359,7 +367,7 @@
                     }).c('body').t(message).up()
                       .c('delay', { xmlns:'urn:xmpp:delay', from: 'localhost', stamp: converse.toISOString(one_day_ago) })
                       .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
-                    this.chatboxes.messageReceived(msg);
+                    this.chatboxes.onMessage(msg);
                     expect(converse.emit).toHaveBeenCalledWith('onMessage', msg);
                     expect(chatbox.messages.length).toEqual(1);
                     msg_obj = chatbox.messages.models[0];
@@ -380,7 +388,7 @@
                         id: new Date().getTime()
                     }).c('body').t(message).up()
                       .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
-                    this.chatboxes.messageReceived(msg);
+                    this.chatboxes.onMessage(msg);
                     expect(converse.emit).toHaveBeenCalledWith('onMessage', msg);
                     // Check that there is a <time> element, with the required
                     // props.
@@ -489,7 +497,7 @@
                         id: (new Date()).getTime()
                     }).c('body').t(message).up()
                       .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
-                this.chatboxes.messageReceived(msg);
+                this.chatboxes.onMessage(msg);
                 expect(converse.incrementMsgCounter).toHaveBeenCalled();
                 expect(this.msg_counter).toBe(1);
                 expect(converse.emit).toHaveBeenCalledWith('onMessage', msg);
@@ -519,7 +527,7 @@
                         id: (new Date()).getTime()
                     }).c('body').t(message).up()
                       .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
-                this.chatboxes.messageReceived(msg);
+                this.chatboxes.onMessage(msg);
                 expect(converse.incrementMsgCounter).not.toHaveBeenCalled();
                 expect(this.msg_counter).toBe(0);
             }, converse));

tests/utils.js

@@ -87,7 +87,7 @@
     };
 
     utils.openChatBoxFor = function (jid) {
-        converse.rosterview.get(jid).openChat(mock.event);
+        return converse.rosterview.get(jid).openChat(mock.event);
     };
 
     utils.clearChatBoxMessages = function (jid) {