Improve how the `muc_domain` setting is populated via disco

Remove brittle code that uses `querySelector` to get the rooms list model.
This code was causing a TypeError due to a race condition.

CHANGES.md

@@ -3,6 +3,7 @@
 ## 9.1.1 (Unreleased)
 
 - GIFs don't render inside unfurls and cause a TypeError
+- Improve how the `muc_domain` setting is populated via service discovery
 - #2746: Always reply to all iqs, even those not understood
 - #2868: Selected emoji is inserted into all open chat boxes
 

karma.conf.js

@@ -69,6 +69,7 @@ module.exports = function(config) {
       { pattern: "src/plugins/muc-views/tests/autocomplete.js", type: 'module' },
       { pattern: "src/plugins/muc-views/tests/component.js", type: 'module' },
       { pattern: "src/plugins/muc-views/tests/corrections.js", type: 'module' },
+      { pattern: "src/plugins/muc-views/tests/disco.js", type: 'module' },
       { pattern: "src/plugins/muc-views/tests/emojis.js", type: 'module' },
       { pattern: "src/plugins/muc-views/tests/hats.js", type: 'module' },
       { pattern: "src/plugins/muc-views/tests/http-file-upload.js", type: 'module' },

src/plugins/muc-views/index.js

@@ -8,7 +8,9 @@ import 'plugins/modal/index.js';
 import './adhoc-commands.js';
 import MUCView from './muc.js';
 import { api, converse } from '@converse/headless/core';
-import { clearHistory, fetchAndSetMUCDomain, parseMessageForMUCCommands } from './utils.js';
+import { clearHistory, parseMessageForMUCCommands } from './utils.js';
+
+const { Strophe } = converse.env;
 
 import './styles/index.scss';
 
@@ -58,6 +60,22 @@ converse.plugins.add('converse-muc-views', {
 
         _converse.ChatRoomView = MUCView;
 
+        if (!api.settings.get('muc_domain')) {
+            // Use service discovery to get the default MUC domain
+            api.listen.on('serviceDiscovered', async (feature) => {
+                if (feature?.get('var') === Strophe.NS.MUC) {
+                    if (feature.entity.get('jid').includes('@')) {
+                        // Ignore full JIDs, we're only looking for a MUC service, not a room
+                        return;
+                    }
+                    const identity = await feature.entity.getIdentity('conference', 'text');
+                    if (identity) {
+                        api.settings.set('muc_domain', Strophe.getDomainFromJid(feature.get('from')));
+                    }
+                }
+            });
+        }
+
         api.listen.on('clearsession', () => {
             const view = _converse.chatboxviews.get('controlbox');
             if (view && view.roomspanel) {
@@ -67,14 +85,6 @@ converse.plugins.add('converse-muc-views', {
             }
         });
 
-        api.listen.on('controlBoxInitialized', view => {
-            if (!api.settings.get('allow_muc')) {
-                return;
-            }
-            fetchAndSetMUCDomain(view);
-            view.model.on('change:connected', () => fetchAndSetMUCDomain(view));
-        });
-
         api.listen.on('chatBoxClosed', (model) => {
             if (model.get('type') === _converse.CHATROOMS_TYPE) {
                 clearHistory(model.get('jid'));

src/plugins/muc-views/modals/muc-list.js

@@ -74,23 +74,23 @@ export default BootstrapModal.extend({
         this.loading_items = false;
 
         BootstrapModal.prototype.initialize.apply(this, arguments);
-        if (api.settings.get('muc_domain') && !this.model.get('muc_domain')) {
-            this.model.save('muc_domain', api.settings.get('muc_domain'));
-        }
         this.listenTo(this.model, 'change:muc_domain', this.onDomainChange);
+        this.listenTo(this.model, 'change:feedback_text', () => this.render());
+
 
         this.el.addEventListener('shown.bs.modal', () => api.settings.get('locked_muc_domain')
           ? this.updateRoomsList()
           : this.el.querySelector('input[name="server"]').focus()
         );
+
+        this.model.save('feedback_text', '');
     },
 
     toHTML () {
-        const muc_domain = this.model.get('muc_domain') || api.settings.get('muc_domain');
         return tpl_muc_list(
             Object.assign(this.model.toJSON(), {
                 'show_form': !api.settings.get('locked_muc_domain'),
-                'server_placeholder': muc_domain ? muc_domain : __('conference.example.org'),
+                'server_placeholder': this.model.get('muc_domain') || __('conference.example.org'),
                 'items': this.items,
                 'loading_items': this.loading_items,
                 'openRoom': ev => this.openRoom(ev),

src/plugins/muc-views/tests/disco.js

@@ -0,0 +1,65 @@
+/*global mock, converse */
+
+describe("Service Discovery", function () {
+
+    it("can be used to set the muc_domain", mock.initConverse( ['discoInitialized'], {}, async function (_converse) {
+        const { u, $iq } = converse.env;
+        const IQ_stanzas = _converse.connection.IQ_stanzas;
+        const IQ_ids =  _converse.connection.IQ_ids;
+        const { api } = _converse;
+
+        expect(api.settings.get('muc_domain')).toBe(undefined);
+
+        await u.waitUntil(() => IQ_stanzas.filter(
+            (iq) => iq.querySelector(`iq[to="montague.lit"] query[xmlns="http://jabber.org/protocol/disco#info"]`)).length > 0
+        );
+
+        let stanza = IQ_stanzas.find((iq) => iq.querySelector('iq[to="montague.lit"] query[xmlns="http://jabber.org/protocol/disco#info"]'));
+        const info_IQ_id = IQ_ids[IQ_stanzas.indexOf(stanza)];
+        stanza = $iq({
+            'type': 'result',
+            'from': 'montague.lit',
+            'to': 'romeo@montague.lit/orchard',
+            'id': info_IQ_id
+        }).c('query', {'xmlns': 'http://jabber.org/protocol/disco#info'})
+            .c('identity', { 'category': 'server', 'type': 'im'}).up()
+            .c('identity', { 'category': 'conference', 'name': 'Play-Specific Chatrooms'}).up()
+            .c('feature', { 'var': 'http://jabber.org/protocol/disco#info'}).up()
+            .c('feature', { 'var': 'http://jabber.org/protocol/disco#items'}).up();
+        _converse.connection._dataRecv(mock.createRequest(stanza));
+
+        const entities = await _converse.api.disco.entities.get();
+        expect(entities.length).toBe(2); // We have an extra entity, which is the user's JID
+        expect(entities.get(_converse.domain).identities.length).toBe(2);
+        expect(entities.get('montague.lit').features.where(
+            {'var': 'http://jabber.org/protocol/disco#items'}).length).toBe(1);
+        expect(entities.get('montague.lit').features.where(
+            {'var': 'http://jabber.org/protocol/disco#info'}).length).toBe(1);
+
+        stanza = await u.waitUntil(() => IQ_stanzas.filter(
+            iq => iq.querySelector('iq[to="montague.lit"] query[xmlns="http://jabber.org/protocol/disco#items"]')).pop()
+        );
+
+        _converse.connection._dataRecv(mock.createRequest($iq({
+            'type': 'result',
+            'from': 'montague.lit',
+            'to': 'romeo@montague.lit/orchard',
+            'id': IQ_ids[IQ_stanzas.indexOf(stanza)]
+        }).c('query', {'xmlns': 'http://jabber.org/protocol/disco#items'})
+            .c('item', { 'jid': 'chat.shakespeare.lit', 'name': 'Chatroom Service'})));
+
+        stanza = await u.waitUntil(() => IQ_stanzas.filter(
+            iq => iq.querySelector('iq[to="chat.shakespeare.lit"] query[xmlns="http://jabber.org/protocol/disco#info"]')).pop()
+        );
+        _converse.connection._dataRecv(mock.createRequest($iq({
+            'type': 'result',
+            'from': 'chat.shakespeare.lit',
+            'to': 'romeo@montague.lit/orchard',
+            'id': IQ_ids[IQ_stanzas.indexOf(stanza)]
+        }).c('query', {'xmlns': 'http://jabber.org/protocol/disco#info'})
+            .c('identity', { 'category': 'conference', 'name': 'Play-Specific Chatrooms', 'type': 'text'}).up()
+            .c('feature', { 'var': 'http://jabber.org/protocol/muc'}).up()));
+
+        await u.waitUntil(() => _converse.api.settings.get('muc_domain') === 'chat.shakespeare.lit');
+    }));
+});

src/plugins/muc-views/utils.js

@@ -61,51 +61,6 @@ export async function destroyMUC (model) {
     }
 }
 
-
-function setMUCDomain (domain, controlboxview) {
-    controlboxview.querySelector('converse-rooms-list')
-        .model.save('muc_domain', Strophe.getDomainFromJid(domain));
-}
-
-function setMUCDomainFromDisco (controlboxview) {
-    /* Check whether service discovery for the user's domain
-     * returned MUC information and use that to automatically
-     * set the MUC domain in the "Add groupchat" modal.
-     */
-    function featureAdded (feature) {
-        if (!feature) {
-            return;
-        }
-        if (feature.get('var') === Strophe.NS.MUC) {
-            feature.entity.getIdentity('conference', 'text').then(identity => {
-                if (identity) {
-                    setMUCDomain(feature.get('from'), controlboxview);
-                }
-            });
-        }
-    }
-    api.waitUntil('discoInitialized')
-        .then(() => {
-            api.listen.on('serviceDiscovered', featureAdded);
-            // Features could have been added before the controlbox was
-            // initialized. We're only interested in MUC
-            _converse.disco_entities.each(entity => featureAdded(entity.features.findWhere({ 'var': Strophe.NS.MUC })));
-        })
-        .catch(e => log.error(e));
-}
-
-export function fetchAndSetMUCDomain (controlboxview) {
-    if (controlboxview.model.get('connected')) {
-        if (!controlboxview.querySelector('converse-rooms-list').model.get('muc_domain')) {
-            if (api.settings.get('muc_domain') === undefined) {
-                setMUCDomainFromDisco(controlboxview);
-            } else {
-                setMUCDomain(api.settings.get('muc_domain'), controlboxview);
-            }
-        }
-    }
-}
-
 export function getNicknameRequiredTemplate (model) {
     const jid = model.get('jid');
     if (api.settings.get('muc_show_logs_before_join')) {

src/plugins/roomslist/index.js

@@ -7,23 +7,17 @@
  */
 import "@converse/headless/plugins/muc/index.js";
 import './view.js';
-import { api, converse } from "@converse/headless/core";
+import { converse } from "@converse/headless/core";
 
 
 converse.plugins.add('converse-roomslist', {
 
-    dependencies: ["converse-singleton", "converse-controlbox", "converse-muc", "converse-bookmarks"],
+    dependencies: [
+        "converse-singleton",
+        "converse-controlbox",
+        "converse-muc",
+        "converse-bookmarks"
+    ],
 
-    initialize () {
-        // Event handlers
-        api.listen.on('connected', async () =>  {
-            if (api.settings.get('allow_bookmarks')) {
-                await api.waitUntil('bookmarksInitialized');
-            } else {
-                await Promise.all([
-                    api.waitUntil('chatBoxesFetched'),
-                ]);
-            }
-        });
-    }
+    initialize () { }
 });

src/plugins/roomslist/model.js

@@ -4,6 +4,7 @@ import { _converse, api, converse } from "@converse/headless/core";
 const { Strophe } = converse.env;
 
 const RoomsListModel = Model.extend({
+
     defaults: function () {
         return {
             'muc_domain': api.settings.get('muc_domain'),
@@ -12,6 +13,10 @@ const RoomsListModel = Model.extend({
         };
     },
 
+    initialize () {
+        api.settings.listen.on('change:muc_domain', (muc_domain) => this.setDomain(muc_domain));
+    },
+
     setDomain (jid) {
         if (!api.settings.get('locked_muc_domain')) {
             this.save('muc_domain', Strophe.getDomainFromJid(jid));

webpack.html

@@ -35,7 +35,7 @@
             // connection_options: { 'worker': '/dist/shared-connection-worker.js' },
             // persistent_store: 'IndexedDB',
             message_archiving: 'always',
-            muc_domain: 'conference.chat.example.org',
+            // muc_domain: 'conference.chat.example.org',
             muc_respect_autojoin: true,
             view_mode: 'fullscreen',
             websocket_url: 'ws://chat.example.org:5380/xmpp-websocket',