Don't automatically show a chat box when creating it. updates #357

This caused a bug whereby a chat box would open only on chat state
notifications.

Also refactored the chats.open and chats.get methods so that they now reuse the
same map function and so that chats.get can now return any chat box and not
just already open ones.

Updated the tests to properly test this and updated the docs.

converse.js

@@ -1016,12 +1016,7 @@
 
                 this.updateVCard();
                 this.$el.insertAfter(converse.chatboxviews.get("controlbox").$el);
-                this.render().model.messages.fetch({add: true});
-                if (this.model.get('minimized')) {
-                    this.hide();
-                } else {
-                    this.show();
-                }
+                this.hide().render().model.messages.fetch({add: true});
                 if ((_.contains([UNVERIFIED, VERIFIED], this.model.get('otr_status'))) || converse.use_otr_by_default) {
                     this.model.initiateOTR();
                 }
@@ -1159,7 +1154,10 @@
                 if ((message.get('sender') != 'me') && (converse.windowState == 'blur')) {
                     converse.incrementMsgCounter();
                 }
-                return this.scrollDown();
+                this.scrollDown();
+                if (!this.model.get('minimized') && !this.$el.is(':visible')) {
+                    this.show();
+                }
             },
 
             sendMessageStanza: function (text) {
@@ -1617,7 +1615,7 @@
                     this.initDragResize();
                 }
                 this.setChatState(ACTIVE);
-                return this;
+                return this.focus();
             },
 
             scrollDown: function () {
@@ -3075,6 +3073,8 @@
             },
 
             onMessage: function (message) {
+                /* Handler method for all incoming single-user chat "message" stanzas.
+                 */
                 var $message = $(message);
                 var contact_jid, $forwarded, $received, $sent,
                     msgid = $message.attr('id'),
@@ -3280,7 +3280,6 @@
             showChat: function (attrs) {
                 /* Find the chat box and show it. If it doesn't exist, create it.
                  */
-                // TODO: Send the chat state ACTIVE to the contact once the chat box is opened.
                 var chatbox  = this.model.get(attrs.jid);
                 if (!chatbox) {
                     chatbox = this.model.create(attrs, {
@@ -5432,6 +5431,7 @@
     var wrappedChatBox = function (chatbox) {
         var view = converse.chatboxviews.get(chatbox.get('jid'));
         return {
+            'open': $.proxy(view.show, view),
             'close': $.proxy(view.close, view),
             'endOTR': $.proxy(chatbox.endOTR, chatbox),
             'focus': $.proxy(view.focus, view),
@@ -5442,6 +5442,27 @@
             'set': $.proxy(chatbox.set, chatbox)
         };
     };
+
+    var getWrappedChatBox = function (jid) {
+        var chatbox = converse.chatboxes.get(jid);
+        if (!chatbox) {
+            var roster_item = converse.roster.get(jid);
+            if (roster_item === undefined) {
+                converse.log('Could not get roster item for JID '+jid, 'error');
+                return null;
+            }
+            chatbox = converse.chatboxes.create({
+                'id': jid,
+                'jid': jid,
+                'fullname': _.isEmpty(roster_item.get('fullname'))? jid: roster_item.get('fullname'),
+                'image_type': roster_item.get('image_type'),
+                'image': roster_item.get('image'),
+                'url': roster_item.get('url')
+            });
+        }
+        return wrappedChatBox(chatbox);
+    };
+
     return {
         'initialize': function (settings, callback) {
             converse.initialize(settings, callback);
@@ -5498,47 +5519,29 @@
         },
         'chats': {
             'open': function (jids) {
-                var _transform = function (jid) {
-                    var chatbox = converse.chatboxes.get(jid);
-                    if (!chatbox) {
-                        var roster_item = converse.roster.get(jid);
-                        if (roster_item === undefined) {
-                            converse.log('Could not get roster item for JID '+jid, 'error');
-                            return null;
-                        }
-                        chatbox = converse.chatboxes.create({
-                            'id': jid,
-                            'jid': jid,
-                            'fullname': _.isEmpty(roster_item.get('fullname'))? jid: roster_item.get('fullname'),
-                            'image_type': roster_item.get('image_type'),
-                            'image': roster_item.get('image'),
-                            'url': roster_item.get('url')
-                        });
-                    }
-                    return wrappedChatBox(chatbox);
-                };
+                var chatbox;
                 if (typeof jids === "undefined") {
                     converse.log("chats.open: You need to provide at least one JID", "error");
                     return null;
                 } else if (typeof jids === "string") {
-                    return _transform(jids);
-                }
-                return _.map(jids, _transform);
+                    chatbox = getWrappedChatBox(jids);
+                    chatbox.open();
+                    return chatbox;
+                }
+                return _.map(jids, function (jid) {
+                    var chatbox = getWrappedChatBox(jid);
+                    chatbox.open();
+                    return chatbox;
+                });
             },
             'get': function (jids) {
-                var _transform = function (jid) {
-                    var chatbox = converse.chatboxes.get(jid);
-                    if (!chatbox) {
-                        return null;
-                    }
-                    return wrappedChatBox(chatbox);
-                };
                 if (typeof jids === "undefined") {
-                    jids = converse.roster.pluck('jid');
+                    converse.log("chats.get: You need to provide at least one JID", "error");
+                    return null;
                 } else if (typeof jids === "string") {
-                    return _transform(jids);
+                    return getWrappedChatBox(jids);
                 }
-                return _.filter(_.map(jids, _transform), function (i) {return i !== null;});
+                return _.map(jids, getWrappedChatBox);
             }
         },
         'tokens': {

docs/CHANGES.rst

@@ -5,7 +5,11 @@ Changelog
 -----------------
 
 * Bugfix. Prevent attaching twice during initialization. [jcbrand]
+* API method chats.get can now also return chat boxes which haven't been opened yet. [jcbrand]
 * Add API method contacts.add. [pzia]
+* #356 Fix the plugin extend function. [floriancargoet]
+* #357 Fix the known bug where a state notification reopens a chat box. [floriancargoet]
+* #358 Bugfix. Chat rooms show the same occupants bug. [floriancargoet]
 
 0.9.1 (2015-03-26)
 ------------------

docs/source/development.rst

@@ -41,9 +41,8 @@ directory:
 
     make dev
 
-On Windows you need to specify Makefile.win to be used by running:
-    
-::
+On Windows you need to specify Makefile.win to be used by running: ::
+
     make -f Makefile.win dev
     
 Or alternatively, if you don't have GNU Make:
@@ -291,8 +290,7 @@ You may also provide the fullname. If not present, we use the jid as fullname.
 get
 ~~~
 
-Returns an object representing a chat box, if that chat box is already open.
-If the chat box is not already open, this method will return ``null``.
+Returns an object representing a chat box.
 
 To return a single chat box, provide the JID of the contact you're chatting
 with in that chat box::
@@ -321,7 +319,7 @@ To return an array of chat boxes, provide an array of JIDs::
     converse.chats.open(['buddy1@example.com', 'buddy2@example.com'])
 
 
-*The returned chat box contains the following methods:*
+*The returned chat box object contains the following methods:*
 
 +-------------+------------------------------------------+
 | Method      | Description                              |
@@ -340,6 +338,8 @@ To return an array of chat boxes, provide an array of JIDs::
 +-------------+------------------------------------------+
 | close       | Close the chat box.                      |
 +-------------+------------------------------------------+
+| open        | Opens the chat box.                      |
++-------------+------------------------------------------+
 
 *The get and set methods can be used to retrieve and change the following attributes:*
 

spec/chatbox.js

@@ -431,7 +431,7 @@
                         this.chatboxes.onMessage(msg);
                         expect(converse.emit).toHaveBeenCalledWith('message', msg);
                     }, converse));
-                    waits(250);
+                    waits(50);
                     runs($.proxy(function () {
                         // Check that the chatbox and its view now exist
                         var chatbox = this.chatboxes.get(sender_jid);
@@ -462,61 +462,46 @@
                     var contact_name = mock.cur_names[0];
                     var contact_jid = contact_name.replace(/ /g,'.').toLowerCase() + '@localhost';
                     spyOn(this, 'emit');
-                    runs(function () {
-                        test_utils.openChatBoxFor(contact_jid);
-                        var chatview = converse.chatboxviews.get(contact_jid);
-                        expect(chatview.model.get('minimized')).toBeFalsy();
-                        chatview.$el.find('.toggle-chatbox-button').click();
-                    });
-                    waits(50);
-                    runs($.proxy(function () {
-                        var chatview = this.chatboxviews.get(contact_jid);
-                        expect(chatview.model.get('minimized')).toBeTruthy();
-                        var message = 'This message is sent to a minimized chatbox';
-                        var sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
-                        msg = $msg({
-                            from: sender_jid,
+                    test_utils.openChatBoxFor(contact_jid);
+                    var chatview = this.chatboxviews.get(contact_jid);
+                    expect(chatview.$el.is(':visible')).toBeTruthy();
+                    expect(chatview.model.get('minimized')).toBeFalsy();
+                    chatview.$el.find('.toggle-chatbox-button').click();
+                    expect(chatview.model.get('minimized')).toBeTruthy();
+                    var message = 'This message is sent to a minimized chatbox';
+                    var sender_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
+                    msg = $msg({
+                        from: sender_jid,
+                        to: this.connection.jid,
+                        type: 'chat',
+                        id: (new Date()).getTime()
+                    }).c('body').t(message).up()
+                    .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
+                    this.chatboxes.onMessage(msg);
+                    expect(this.emit).toHaveBeenCalledWith('message', msg);
+                    var trimmed_chatboxes = this.minimized_chats;
+                    var trimmedview = trimmed_chatboxes.get(contact_jid);
+                    var $count = trimmedview.$el.find('.chat-head-message-count');
+                    expect(chatview.$el.is(':visible')).toBeFalsy();
+                    expect(trimmedview.model.get('minimized')).toBeTruthy();
+                    expect($count.is(':visible')).toBeTruthy();
+                    expect($count.html()).toBe('1');
+                    this.chatboxes.onMessage(
+                        $msg({
+                            from: mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost',
                             to: this.connection.jid,
                             type: 'chat',
                             id: (new Date()).getTime()
-                        }).c('body').t(message).up()
-                        .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree();
-                        this.chatboxes.onMessage(msg);
-                        expect(this.emit).toHaveBeenCalledWith('message', msg);
-                    }, converse));
-                    waits(50);
-                    runs($.proxy(function () {
-                        var trimmed_chatboxes = this.minimized_chats;
-                        var trimmedview = trimmed_chatboxes.get(contact_jid);
-                        var $count = trimmedview.$el.find('.chat-head-message-count');
-                        expect(trimmedview.model.get('minimized')).toBeTruthy();
-                        expect($count.is(':visible')).toBeTruthy();
-                        expect($count.html()).toBe('1');
-                        this.chatboxes.onMessage(
-                            $msg({
-                                from: mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost',
-                                to: this.connection.jid,
-                                type: 'chat',
-                                id: (new Date()).getTime()
-                            }).c('body').t('This message is also sent to a minimized chatbox').up()
-                            .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree()
-                        );
-                    }, converse));
-                    waits(50);
-                    runs($.proxy(function () {
-                        var trimmed_chatboxes = this.minimized_chats;
-                        var trimmedview = trimmed_chatboxes.get(contact_jid);
-                        var $count = trimmedview.$el.find('.chat-head-message-count');
-                        expect(trimmedview.model.get('minimized')).toBeTruthy();
-                        expect($count.is(':visible')).toBeTruthy();
-                        expect($count.html()).toBe('2');
-                        trimmedview.$el.find('.restore-chat').click();
-                    }, converse));
-                    waits(250);
-                    runs($.proxy(function () {
-                        var trimmed_chatboxes = this.minimized_chats;
-                        expect(trimmed_chatboxes.keys().length).toBe(0);
-                    }, converse));
+                        }).c('body').t('This message is also sent to a minimized chatbox').up()
+                        .c('active', {'xmlns': 'http://jabber.org/protocol/chatstates'}).tree()
+                    );
+                    expect(chatview.$el.is(':visible')).toBeFalsy();
+                    expect(trimmedview.model.get('minimized')).toBeTruthy();
+                    $count = trimmedview.$el.find('.chat-head-message-count');
+                    expect($count.is(':visible')).toBeTruthy();
+                    expect($count.html()).toBe('2');
+                    trimmedview.$el.find('.restore-chat').click();
+                    expect(trimmed_chatboxes.keys().length).toBe(0);
                 }, converse));
 
                 it("will indicate when it has a time difference of more than a day between it and its predecessor", $.proxy(function () {
@@ -591,21 +576,16 @@
                 it("can be sent from a chatbox, and will appear inside it", $.proxy(function () {
                     spyOn(converse, 'emit');
                     var contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
-                    runs(function () {
-                        test_utils.openChatBoxFor(contact_jid);
-                    });
-                    waits(250);
-                    runs(function () {
-                        expect(converse.emit).toHaveBeenCalledWith('chatBoxFocused', jasmine.any(Object));
-                        var view = this.chatboxviews.get(contact_jid);
-                        var message = 'This message is sent from this chatbox';
-                        spyOn(view, 'sendMessage').andCallThrough();
-                        test_utils.sendMessage(view, message);
-                        expect(view.sendMessage).toHaveBeenCalled();
-                        expect(view.model.messages.length, 2);
-                        expect(converse.emit.mostRecentCall.args, ['messageSend', message]);
-                        expect(view.$el.find('.chat-content').find('.chat-message').last().find('.chat-message-content').text()).toEqual(message);
-                    }.bind(converse));
+                    test_utils.openChatBoxFor(contact_jid);
+                    expect(converse.emit).toHaveBeenCalledWith('chatBoxFocused', jasmine.any(Object));
+                    var view = this.chatboxviews.get(contact_jid);
+                    var message = 'This message is sent from this chatbox';
+                    spyOn(view, 'sendMessage').andCallThrough();
+                    test_utils.sendMessage(view, message);
+                    expect(view.sendMessage).toHaveBeenCalled();
+                    expect(view.model.messages.length, 2);
+                    expect(converse.emit.mostRecentCall.args, ['messageSend', message]);
+                    expect(view.$el.find('.chat-content').find('.chat-message').last().find('.chat-message-content').text()).toEqual(message);
                 }, converse));
 
                 it("is sanitized to prevent Javascript injection attacks", $.proxy(function () {
@@ -697,6 +677,23 @@
 
             describe("A Chat Status Notification", $.proxy(function () {
 
+                it("does not open automatically if a chat state notification is received", $.proxy(function () {
+                    spyOn(converse, 'emit');
+                    var sender_jid = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@localhost';
+                    // <composing> state
+                    var msg = $msg({
+                            from: sender_jid,
+                            to: this.connection.jid,
+                            type: 'chat',
+                            id: (new Date()).getTime()
+                        }).c('body').c('composing', {'xmlns': Strophe.NS.CHATSTATES}).tree();
+                    this.chatboxes.onMessage(msg);
+                    expect(converse.emit).toHaveBeenCalledWith('message', msg);
+                    var chatboxview = this.chatboxviews.get(sender_jid);
+                    expect(chatboxview).toBeDefined();
+                    expect(chatboxview.$el.is(':visible')).toBeFalsy(); // The chat box is not visible
+                }, converse));
+
                 describe("An active notification", $.proxy(function () {
                     it("is sent when the user opens a chat box", $.proxy(function () {
                         spyOn(converse.connection, 'send');

spec/converse.js

@@ -137,21 +137,23 @@
                 test_utils.createContacts('current');
             }, converse));
 
-            it("has a method 'get' which returns a wrapped chat box if it's already open", $.proxy(function () {
+            it("has a method 'get' which returns a wrapped chat box", $.proxy(function () {
                 // Test on chat that doesn't exist.
                 expect(converse_api.chats.get('non-existing@jabber.org')).toBeFalsy();
 
                 // Test on chat that's not open
                 var jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
                 var box = converse_api.chats.get(jid);
-                expect(box).toBe(null);
+                expect(box instanceof Object).toBeTruthy();
+                var chatboxview = this.chatboxviews.get(jid);
+                expect(chatboxview.$el.is(':visible')).toBeFalsy();
 
                 // Test for single JID
                 test_utils.openChatBoxFor(jid);
                 box = converse_api.chats.get(jid);
                 expect(box instanceof Object).toBeTruthy();
                 expect(box.get('box_id')).toBe(b64_sha1(jid));
-                var chatboxview = this.chatboxviews.get(jid);
+                chatboxview = this.chatboxviews.get(jid);
                 expect(chatboxview.$el.is(':visible')).toBeTruthy();
 
                 // Test for multiple JIDs