Fetch the room information before opening the room.

docs/CHANGES.md

@@ -2,9 +2,8 @@
 
 ## 2.0.4 (Unreleased)
 - #737: Bugfix. Translations weren't being applied. [jcbrand]
-- Room information as returned by the server is now stored on the room model.
-  For context, see: http://xmpp.org/extensions/xep-0045.html#disco-roominfo
-  [jcbrand]
+- Fetch room info and store it on the room model.
+  For context, see: http://xmpp.org/extensions/xep-0045.html#disco-roominfo [jcbrand]
 - Bugfix. Switching from bookmarks form to config form shows only the spinner. [jcbrand]
 - Bugfix. Other room occupants sometimes not shown when reloading the page. [jcbrand]
 - Bugfix. Due to changes in `converse-core` the controlbox wasn't aware anymore of

spec/chatroom.js

@@ -160,6 +160,17 @@
                             'whois': 'anyone'
                         }
                     });
+
+                    // We pretend this is a new room, so no disco info is returned.
+                    var features_stanza = $iq({
+                            from: 'room@conference.example.org',
+                            'id': IQ_id,
+                            'to': 'dummy@localhost/desktop',
+                            'type': 'error'
+                        }).c('error', {'type': 'cancel'})
+                            .c('item-not-found', {'xmlns': "urn:ietf:params:xml:ns:xmpp-stanzas"});
+                    converse.connection._dataRecv(test_utils.createRequest(features_stanza));
+
                     /* <presence xmlns="jabber:client" to="dummy@localhost/pda" from="room@conference.example.org/yo">
                      *  <x xmlns="http://jabber.org/protocol/muc#user">
                      *      <item affiliation="owner" jid="dummy@localhost/pda" role="moderator"/>
@@ -251,11 +262,20 @@
                     sent_IQ = iq;
                     IQ_id = sendIQ.bind(this)(iq, callback, errback);
                 });
-
                 runs(function () {
                     converse_api.rooms.open('coven@chat.shakespeare.lit', {'nick': 'some1'});
                     view = converse.chatboxviews.get('coven@chat.shakespeare.lit');
-                    spyOn(view, 'findAndSaveOwnAffiliation').andCallThrough();
+                    spyOn(view, 'saveAffiliationAndRole').andCallThrough();
+
+                    // We pretend this is a new room, so no disco info is returned.
+                    var features_stanza = $iq({
+                            from: 'coven@chat.shakespeare.lit',
+                            'id': IQ_id,
+                            'to': 'dummy@localhost/desktop',
+                            'type': 'error'
+                        }).c('error', {'type': 'cancel'})
+                            .c('item-not-found', {'xmlns': "urn:ietf:params:xml:ns:xmpp-stanzas"});
+                    converse.connection._dataRecv(test_utils.createRequest(features_stanza));
 
                     /* <presence to="dummy@localhost/converse.js-29092160"
                      *           from="coven@chat.shakespeare.lit/some1">
@@ -276,7 +296,7 @@
                         }).up()
                         .c('status', {code: '110'});
                     converse.connection._dataRecv(test_utils.createRequest(presence));
-                    expect(view.findAndSaveOwnAffiliation).toHaveBeenCalled();
+                    expect(view.saveAffiliationAndRole).toHaveBeenCalled();
                     expect(view.$('.configure-chatroom-button').is(':visible')).toBeTruthy();
                     expect(view.$('.toggle-chatbox-button').is(':visible')).toBeTruthy();
                     expect(view.$('.toggle-bookmark').is(':visible')).toBeTruthy();
@@ -303,7 +323,7 @@
                     /* Server responds with the configuration form.
                      * See: // http://xmpp.org/extensions/xep-0045.html#example-165
                      */
-                     var config_stanza = $iq({from: 'conven@chat.shakespeare.lit',
+                     var config_stanza = $iq({from: 'coven@chat.shakespeare.lit',
                           'id': IQ_id,
                           'to': 'dummy@localhost/desktop',
                           'type': 'result'})
@@ -535,6 +555,17 @@
                 });
 
                 test_utils.openChatRoom(converse, 'lounge', 'localhost', 'dummy');
+
+                // We pretend this is a new room, so no disco info is returned.
+                var features_stanza = $iq({
+                        from: 'lounge@localhost',
+                        'id': IQ_id,
+                        'to': 'dummy@localhost/desktop',
+                        'type': 'error'
+                    }).c('error', {'type': 'cancel'})
+                        .c('item-not-found', {'xmlns': "urn:ietf:params:xml:ns:xmpp-stanzas"});
+                converse.connection._dataRecv(test_utils.createRequest(features_stanza));
+
                 var view = converse.chatboxviews.get('lounge@localhost');
                 spyOn(view, 'join').andCallThrough();
 
@@ -627,13 +658,15 @@
             }));
 
             it("can be joined automatically, based upon a received invite", mock.initConverse(function (converse) {
-                test_utils.openChatRoom(converse, 'lounge', 'localhost', 'dummy');
+                test_utils.createContacts(converse, 'current'); // We need roster contacts, who can invite us
                 spyOn(window, 'confirm').andCallFake(function () {
                     return true;
                 });
-                test_utils.createContacts(converse, 'current'); // We need roster contacts, who can invite us
+                test_utils.openAndEnterChatRoom(converse, 'lounge', 'localhost', 'dummy');
                 var view = converse.chatboxviews.get('lounge@localhost');
                 view.close();
+                view.model.destroy(); // Manually calling this, otherwise we have to mock stanzas.
+
                 var name = mock.cur_names[0];
                 var from_jid = name.replace(/ /g,'.').toLowerCase() + '@localhost';
                 var room_jid = 'lounge@localhost';
@@ -874,23 +907,95 @@
                 expect($occupants.children().first(0).text()).toBe("newnick");
             }));
 
+            if("queries for the room information before attempting to join the user",  mock.initConverse(function (converse) {
+                var sent_IQ, IQ_id;
+                var sendIQ = converse.connection.sendIQ;
+                spyOn(converse.connection, 'sendIQ').andCallFake(function (iq, callback, errback) {
+                    sent_IQ = iq;
+                    IQ_id = sendIQ.bind(this)(iq, callback, errback);
+                });
+
+                converse_api.rooms.open('coven@chat.shakespeare.lit', {'nick': 'some1'});
+
+                // Check that the room queried for the feautures.
+                expect(sent_IQ.toLocaleString()).toBe(
+                    "<iq from='dummy@localhost/resource' to='coven@chat.shakespeare.lit' type='get' xmlns='jabber:client' id='"+IQ_id+"'>"+
+                        "<query xmlns='http://jabber.org/protocol/disco#info'/>"+
+                    "</iq>");
+
+                /* <iq from='coven@chat.shakespeare.lit'
+                 *      id='ik3vs715'
+                 *      to='hag66@shakespeare.lit/pda'
+                 *      type='result'>
+                 *  <query xmlns='http://jabber.org/protocol/disco#info'>
+                 *      <identity
+                 *          category='conference'
+                 *          name='A Dark Cave'
+                 *          type='text'/>
+                 *      <feature var='http://jabber.org/protocol/muc'/>
+                 *      <feature var='muc_passwordprotected'/>
+                 *      <feature var='muc_hidden'/>
+                 *      <feature var='muc_temporary'/>
+                 *      <feature var='muc_open'/>
+                 *      <feature var='muc_unmoderated'/>
+                 *      <feature var='muc_nonanonymous'/>
+                 *  </query>
+                 *  </iq>
+                 */
+                var features_stanza = $iq({
+                        from: 'coven@chat.shakespeare.lit',
+                        'id': IQ_id,
+                        'to': 'dummy@localhost/desktop',
+                        'type': 'result'
+                    })
+                    .c('query', { 'xmlns': 'http://jabber.org/protocol/disco#info'})
+                        .c('identity', {
+                            'category': 'conference',
+                            'name': 'A Dark Cave',
+                            'type': 'text'
+                        }).up()
+                        .c('feature', {'var': 'http://jabber.org/protocol/muc'}).up()
+                        .c('feature', {'var': 'passwordprotected'}).up()
+                        .c('feature', {'var': 'hidden'}).up()
+                        .c('feature', {'var': 'temporary'}).up()
+                        .c('feature', {'var': 'open'}).up()
+                        .c('feature', {'var': 'unmoderated'}).up()
+                        .c('feature', {'var': 'nonanonymous'});
+                converse.connection._dataRecv(test_utils.createRequest(features_stanza));
+
+                var view = converse.chatboxviews.get('coven@chat.shakespeare.lit');
+                expect(view.model.get('passwordprotected')).toBe('true');
+                expect(view.model.get('hidden')).toBe('true');
+                expect(view.model.get('temporary')).toBe('true');
+                expect(view.model.get('open')).toBe('true');
+                expect(view.model.get('unmoderated')).toBe('true');
+                expect(view.model.get('nonanonymous')).toBe('true');
+            }));
+
             it("indicates when a room is no longer anonymous", mock.initConverse(function (converse) {
-                converse_api.rooms.open('room@conference.example.org', {
-                    'nick': 'some1',
-                    'auto_configure': true,
-                    'roomconfig': {
-                        'changesubject': false,
-                        'membersonly': true,
-                        'persistentroom': true,
-                        'publicroom': true,
-                        'roomdesc': 'Welcome to this room',
-                        'whois': 'anyone'
-                    }
+                var sent_IQ, IQ_id;
+                var sendIQ = converse.connection.sendIQ;
+                spyOn(converse.connection, 'sendIQ').andCallFake(function (iq, callback, errback) {
+                    sent_IQ = iq;
+                    IQ_id = sendIQ.bind(this)(iq, callback, errback);
                 });
+                converse_api.rooms.open('coven@chat.shakespeare.lit', {'nick': 'some1'});
+
+                // We pretend this is a new room, so no disco info is returned.
+                var features_stanza = $iq({
+                        from: 'coven@chat.shakespeare.lit',
+                        'id': IQ_id,
+                        'to': 'dummy@localhost/desktop',
+                        'type': 'error'
+                    }).c('error', {'type': 'cancel'})
+                        .c('item-not-found', {'xmlns': "urn:ietf:params:xml:ns:xmpp-stanzas"});
+                converse.connection._dataRecv(test_utils.createRequest(features_stanza));
+
+                var view = converse.chatboxviews.get('coven@chat.shakespeare.lit');
                 /* <message xmlns="jabber:client"
                  *              type="groupchat"
                  *              to="dummy@localhost/converse.js-27854181"
-                 *              from="room@conference.example.org">
+                 *              from="coven@chat.shakespeare.lit">
                  *      <x xmlns="http://jabber.org/protocol/muc#user">
                  *          <status code="104"/>
                  *          <status code="172"/>
@@ -900,12 +1005,11 @@
                 var message = $msg({
                         type:'groupchat',
                         to: 'dummy@localhost/converse.js-27854181',
-                        from: 'room@conference.example.org'
+                        from: 'coven@chat.shakespeare.lit'
                     }).c('x', {xmlns: Strophe.NS.MUC_USER})
                       .c('status', {code: '104'}).up()
                       .c('status', {code: '172'});
                 converse.connection._dataRecv(test_utils.createRequest(message));
-                var view = converse.chatboxviews.get('room@conference.example.org');
                 var $chat_body = view.$('.chatroom-body');
                 expect($chat_body.html().trim().indexOf(
                     '<div class="chat-info">This room is now no longer anonymous</div>'
@@ -1029,7 +1133,11 @@
                 runs(function () {
                     expect(view.close).toHaveBeenCalled();
                     expect(view.leave).toHaveBeenCalled();
-                    expect(converse.emit).toHaveBeenCalledWith('chatBoxClosed', jasmine.any(Object));
+                    // XXX: After refactoring, the chat box only gets closed
+                    // once we have confirmation from the server. To test this,
+                    // we would have to mock the returned presence stanza.
+                    // See the "leave" method on the ChatRoomView.
+                    // expect(converse.emit).toHaveBeenCalledWith('chatBoxClosed', jasmine.any(Object));
                 });
             }));
         });
@@ -1174,18 +1282,17 @@
             }));
 
             it("will automatically choose a new nickname if a nickname conflict happens and muc_nickname_from_jid=true", mock.initConverse(function (converse) {
-                /*
-                    <presence
-                        from='coven@chat.shakespeare.lit/thirdwitch'
-                        id='n13mt3l'
-                        to='hag66@shakespeare.lit/pda'
-                        type='error'>
-                    <x xmlns='http://jabber.org/protocol/muc'/>
-                    <error by='coven@chat.shakespeare.lit' type='cancel'>
-                        <conflict xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>
-                    </error>
-                    </presence>
-                */
+                /* <presence
+                 *      from='coven@chat.shakespeare.lit/thirdwitch'
+                 *      id='n13mt3l'
+                 *      to='hag66@shakespeare.lit/pda'
+                 *      type='error'>
+                 *  <x xmlns='http://jabber.org/protocol/muc'/>
+                 *  <error by='coven@chat.shakespeare.lit' type='cancel'>
+                 *      <conflict xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>
+                 *  </error>
+                 *  </presence>
+                 */
                 submitRoomForm(converse);
                 converse.muc_nickname_from_jid = true;
 

src/converse-bookmarks.js

@@ -104,7 +104,7 @@
                     if (!_.isUndefined(model) && model.get('nick')) {
                         this.join(this.model.get('nick'));
                     } else {
-                        this.__super__.checkForReservedNick.apply(this, arguments);
+                        return this.__super__.checkForReservedNick.apply(this, arguments);
                     }
                 },
 

src/converse-chatview.js

@@ -662,7 +662,6 @@
                         // model is going to be destroyed afterwards.
                         this.model.set('chat_state', converse.INACTIVE);
                         this.sendChatState();
-
                         this.model.destroy();
                     }
                     this.remove();

src/converse-muc.js

@@ -310,7 +310,21 @@
                 return converse.chatboxviews.showChat(
                     _.extend(settings, {
                         'type': 'chatroom',
-                        'affiliation': null
+                        'affiliation': null,
+                        'features_fetched': false,
+                        'hidden': false,
+                        'membersonly': false,
+                        'moderated': false,
+                        'nonanonymous': false,
+                        'open': false,
+                        'passwordprotected': false,
+                        'persistent': false,
+                        'public': false,
+                        'semianonymous': false,
+                        'temporary': false,
+                        'unmoderated': false,
+                        'unsecured': false,
+                        'connection_status': Strophe.Status.DISCONNECTED
                     })
                 );
             };
@@ -337,6 +351,7 @@
                 },
 
                 initialize: function () {
+                    var that = this;
                     this.model.messages.on('add', this.onMessageAdded, this);
                     this.model.on('show', this.show, this);
                     this.model.on('destroy', this.hide, this);
@@ -345,22 +360,18 @@
                     this.model.on('change:name', this.renderHeading, this);
 
                     this.createOccupantsView();
-                    this.render();
-                    var nick = this.model.get('nick');
-                    if (!nick) {
-                        this.checkForReservedNick();
-                    } else {
-                        this.join(nick);
-                    }
-
-                    this.fetchMessages().insertIntoDOM();
-                    // XXX: adding the event below to the events map above doesn't work.
+                    this.render().insertIntoDOM(); // TODO: hide chat area until messages received.
+                    // XXX: adding the event below to the declarative events map doesn't work.
                     // The code that gets executed because of that looks like this:
                     //      this.$el.on('scroll', '.chat-content', this.markScrolled.bind(this));
                     // Which for some reason doesn't work.
                     // So working around that fact here:
                     this.$el.find('.chat-content').on('scroll', this.markScrolled.bind(this));
-                    converse.emit('chatRoomOpened', this);
+
+                    this.getRoomFeatures().always(function () {
+                        that.join().fetchMessages();
+                        converse.emit('chatRoomOpened', that);
+                    });
                 },
 
                 createOccupantsView: function () {
@@ -447,7 +458,6 @@
                      * well.
                      */
                     this.leave();
-                    converse.ChatBoxView.prototype.close.apply(this, arguments);
                 },
 
                 toggleOccupants: function (ev, preserve_state) {
@@ -763,7 +773,7 @@
                     var room_now_fully_anon = stanza.querySelector("status[code='173']");
                     if (configuration_changed || logging_enabled || logging_disabled ||
                             room_no_longer_anon || room_now_semi_anon || room_now_fully_anon) {
-                        this.cacheRoomFeatures();
+                        this.getRoomFeatures();
                     }
                     _.compose(this.onChatRoomMessage.bind(this), this.showStatusMessages.bind(this))(stanza);
                     return true;
@@ -830,6 +840,10 @@
                      *  (String) password: Optional password, if required by
                      *      the room.
                      */
+                    nick = nick ? nick : this.model.get('nick');
+                    if (!nick) {
+                        return this.checkForReservedNick();
+                    }
                     this.registerHandlers();
                     if (this.model.get('connection_status') ===  Strophe.Status.CONNECTED) {
                         // We have restored a chat room from session storage,
@@ -845,12 +859,14 @@
                         stanza.cnode(Strophe.xmlElement("password", [], password));
                     }
                     this.model.save('connection_status', Strophe.Status.CONNECTING);
-                    return converse.connection.send(stanza);
+                    converse.connection.send(stanza);
+                    return this;
                 },
 
                 cleanup: function () {
                     this.model.save('connection_status', Strophe.Status.DISCONNECTED);
                     this.removeHandlers();
+                    converse.ChatBoxView.prototype.close.apply(this, arguments);
                 },
 
                 leave: function(exit_msg) {
@@ -863,7 +879,8 @@
                     this.occupantsview.model.reset();
                     this.occupantsview.model.browserStorage._clear();
 
-                    if (!converse.connection.connected) {
+                    if (!converse.connection.connected ||
+                            this.model.get('connection_status') === Strophe.Status.DISCONNECTED) {
                         // Don't send out a stanza if we're not connected.
                         this.cleanup();
                         return;
@@ -1058,45 +1075,45 @@
                     return deferred.promise();
                 },
 
-                cacheRoomFeatures: function () {
+                getRoomFeatures: function () {
                     /* Fetch the room disco info, parse it and then
                      * save it on the Backbone.Model of this chat rooms.
-                     *
-                     * See http://xmpp.org/extensions/xep-0045.html#disco-roominfo
                      */
+                    var deferred = new $.Deferred();
                     var that = this;
                     converse.connection.disco.info(this.model.get('jid'), null,
                         function (iq) {
-                            /* <iq from='coven@chat.shakespeare.lit'
-                             *      id='ik3vs715'
-                             *      to='hag66@shakespeare.lit/pda'
-                             *      type='result'>
-                             *  <query xmlns='http://jabber.org/protocol/disco#info'>
-                             *      <identity
-                             *          category='conference'
-                             *          name='A Dark Cave'
-                             *          type='text'/>
-                             *      <feature var='http://jabber.org/protocol/muc'/>
-                             *      <feature var='muc_passwordprotected'/>
-                             *      <feature var='muc_hidden'/>
-                             *      <feature var='muc_temporary'/>
-                             *      <feature var='muc_open'/>
-                             *      <feature var='muc_unmoderated'/>
-                             *      <feature var='muc_nonanonymous'/>
-                             *  </query>
-                             *  </iq>
+                            /* 
+                             * See http://xmpp.org/extensions/xep-0045.html#disco-roominfo
+                             *
+                             *  <identity
+                             *      category='conference'
+                             *      name='A Dark Cave'
+                             *      type='text'/>
+                             *  <feature var='http://jabber.org/protocol/muc'/>
+                             *  <feature var='muc_passwordprotected'/>
+                             *  <feature var='muc_hidden'/>
+                             *  <feature var='muc_temporary'/>
+                             *  <feature var='muc_open'/>
+                             *  <feature var='muc_unmoderated'/>
+                             *  <feature var='muc_nonanonymous'/>
                              */
-                            var features = [];
+                            var features = {
+                                'features_fetched': true
+                            };
                             _.each(iq.querySelectorAll('feature'), function (field) {
                                 var fieldname = field.getAttribute('var');
                                 if (!fieldname.startsWith('muc_')) {
                                     return;
                                 }
-                                features.push(fieldname.replace('muc_', ''));
+                                features[fieldname.replace('muc_', '')] = true;
                             });
-                            that.model.save({'features': features});
-                        }
+                            that.model.save(features);
+                            return deferred.resolve();
+                        },
+                        deferred.reject
                     );
+                    return deferred.promise();
                 },
 
                 configureChatRoom: function (ev) {
@@ -1163,6 +1180,7 @@
                         this.onNickNameFound.bind(this),
                         this.onNickNameNotFound.bind(this)
                     );
+                    return this;
                 },
 
                 onNickNameFound: function (iq) {
@@ -1305,7 +1323,7 @@
                     return;
                 },
 
-                findAndSaveOwnAffiliation: function (pres) {
+                saveAffiliationAndRole: function (pres) {
                     /* Parse the presence stanza for the current user's
                      * affiliation.
                      *
@@ -1320,13 +1338,17 @@
                     // then the Sizzle selector library might still be needed
                     // here.
                     var item = $(pres).find('x[xmlns="'+Strophe.NS.MUC_USER+'"] item').get(0);
-                    if (_.isUndefined(item)) {
-                        return;
-                    }
+                    if (_.isUndefined(item)) { return; }
                     var jid = item.getAttribute('jid');
-                    var affiliation = item.getAttribute('affiliation');
-                    if (Strophe.getBareJidFromJid(jid) === converse.bare_jid && affiliation) {
-                        this.model.save({'affiliation': affiliation});
+                    if (Strophe.getBareJidFromJid(jid) === converse.bare_jid) {
+                        var affiliation = item.getAttribute('affiliation');
+                        var role = item.getAttribute('role');
+                        if (affiliation) {
+                            this.model.save({'affiliation': affiliation});
+                        }
+                        if (role) {
+                            this.model.save({'role': role});
+                        }
                     }
                 },
 
@@ -1480,7 +1502,7 @@
                      *
                      * See http://xmpp.org/extensions/xep-0045.html#createroom-instant
                      */
-                    this.sendConfiguration().then(this.cacheRoomFeatures.bind(this));
+                    this.sendConfiguration().then(this.getRoomFeatures.bind(this));
                 },
 
                 onChatRoomPresence: function (pres) {
@@ -1499,7 +1521,7 @@
                     var new_room = pres.querySelector("status[code='201']");
 
                     if (is_self) {
-                        this.findAndSaveOwnAffiliation(pres);
+                        this.saveAffiliationAndRole(pres);
                     }
                     if (is_self && new_room) {
                         // This is a new room. It will now be configured
@@ -1515,17 +1537,22 @@
                                 show_status_messages = false;
                             }
                         }
-                    } else if (this.model.get('connection_status') !== Strophe.Status.CONNECTED) {
-                        // This is not a new room, and this is the first
-                        // presence received or the room config has
-                        // changed.
-                        this.cacheRoomFeatures();
+                    } else if (!this.model.get('features_fetched') &&
+                                    this.model.get('connection_status') !== Strophe.Status.CONNECTED) {
+                        // The features for this room weren't fetched yet, perhaps
+                        // because it's a new room without locking (in which
+                        // case Prosody doesn't send a 201 status).
+                        // This is the first presence received for the room, so
+                        // a good time to fetch the features.
+                        this.getRoomFeatures();
                     }
                     if (show_status_messages) {
                         this.hideSpinner().showStatusMessages(pres);
                     }
                     this.occupantsview.updateOccupantsOnPresence(pres);
-                    this.model.save('connection_status', Strophe.Status.CONNECTED);
+                    if (this.model.get('role') !== 'none') {
+                        this.model.save('connection_status', Strophe.Status.CONNECTED);
+                    }
                     return true;
                 },
 
@@ -2211,7 +2238,7 @@
                 converse.chatboxviews.each(function (view) {
                     if (view.model.get('type') === 'chatroom') {
                         view.model.save('connection_status', Strophe.Status.DISCONNECTED);
-                        view.join(view.model.get('nick'));
+                        view.join();
                     }
                 });
             };

tests/utils.js

@@ -122,6 +122,16 @@
         utils.openChatRoom(converse, room, server);
         var view = converse.chatboxviews.get(room+'@'+server);
 
+        // We pretend this is a new room, so no disco info is returned.
+        var features_stanza = $iq({
+                from: 'lounge@localhost',
+                'id': IQ_id,
+                'to': 'dummy@localhost/desktop',
+                'type': 'error'
+            }).c('error', {'type': 'cancel'})
+                .c('item-not-found', {'xmlns': "urn:ietf:params:xml:ns:xmpp-stanzas"});
+        converse.connection._dataRecv(utils.createRequest(features_stanza));
+
         // The XMPP server returns the reserved nick for this user.
         var stanza = $iq({
             'type': 'result',