Properly test and fix show/hide of MUC topic

CHANGES.md

@@ -18,6 +18,7 @@ Soon we'll deprecate the latter, so prepare now.
 - #1826: A user can now add himself as a contact
 - #1839: Headline messages are shown in controlbox
 - #1896: Don't send receipts for messages fetched from the archive 
+- #1937: Editing a message removes the mentions highlight
 - Allow ignoring of bootstrap modules at build using environment variable. For xample: `export BOOTSTRAP_IGNORE_MODULES="Modal,Dropdown" && make dist`
 - Bugfix. Handle stanza that clears the MUC subject
 - New config option [modtools_disable_assign](https://conversejs.org/docs/html/configuration.html#modtools-disable-assign)
@@ -26,7 +27,6 @@ Soon we'll deprecate the latter, so prepare now.
 - Start using [lit-html](https://lit-html.polymer-project.org/) instead of lodash for templating.
 - [muc_fetch_members](https://conversejs.org/docs/html/configuration.html#muc-fetch-members) now also accepts an array of affiliations to fetch.
 - Remove the configuration setting `muc_show_join_leave_status`. The optional status message is no longer shown at all.
-- #1937: Editing a message removes the mentions highlight
 
 ## 6.0.0 (2020-01-09)
 

sass/_chatbox.scss

@@ -57,10 +57,10 @@
             font-size: var(--font-size-small);
             margin: 0;
             overflow: hidden;
-            padding: 0.5rem 1rem 1rem 1rem;
+            padding: 0.5rem 1rem 0.5rem 1rem;
             text-overflow: ellipsis;
-            white-space: nowrap;
             width: 100%;
+            max-height: 5em;
         }
 
         .chatbox-title {

spec/bookmarks.js

@@ -37,7 +37,7 @@
             spyOn(view, 'renderBookmarkForm').and.callThrough();
             spyOn(view, 'closeForm').and.callThrough();
             await u.waitUntil(() => view.el.querySelector('.toggle-bookmark') !== null);
-            let toggle = view.el.querySelector('.toggle-bookmark');
+            const toggle = view.el.querySelector('.toggle-bookmark');
             expect(toggle.title).toBe('Bookmark this groupchat');
             toggle.click();
             expect(view.renderBookmarkForm).toHaveBeenCalled();
@@ -130,10 +130,9 @@
             });
             _converse.connection._dataRecv(test_utils.createRequest(stanza));
             await u.waitUntil(() => view.model.get('bookmarked'));
-            toggle = await u.waitUntil(() => view.el.querySelector('.toggle-bookmark'));
             expect(view.model.get('bookmarked')).toBeTruthy();
-            expect(toggle.title).toBe('Unbookmark this groupchat');
-            expect(u.hasClass('on-button', toggle), true);
+            await u.waitUntil(() => view.el.querySelector('.toggle-bookmark')?.title === 'Unbookmark this groupchat');
+            expect(u.hasClass('on-button', view.el.querySelector('.toggle-bookmark')), true);
             // We ignore this IQ stanza... (unless it's an error stanza), so
             // nothing to test for here.
             done();

spec/muc.js

@@ -501,6 +501,100 @@
                 });
             });
 
+            describe("the topic", function () {
+
+                it("is shown the header",
+                    mock.initConverse(
+                        ['rosterGroupsFetched'], {},
+                        async function (done, _converse) {
+
+                    await test_utils.openAndEnterChatRoom(_converse, 'jdev@conference.jabber.org', 'jc');
+                    const text = 'Jabber/XMPP Development | RFCs and Extensions: https://xmpp.org/ | Protocol and XSF discussions: xsf@muc.xmpp.org';
+                    let stanza = u.toStanza(`
+                        <message xmlns="jabber:client" to="jc@opkode.com/_converse.js-60429116" type="groupchat" from="jdev@conference.jabber.org/ralphm">
+                            <subject>${text}</subject>
+                            <delay xmlns="urn:xmpp:delay" stamp="2014-02-04T09:35:39Z" from="jdev@conference.jabber.org"/>
+                            <x xmlns="jabber:x:delay" stamp="20140204T09:35:39" from="jdev@conference.jabber.org"/>
+                        </message>`);
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                    const view = _converse.chatboxviews.get('jdev@conference.jabber.org');
+                    await new Promise(resolve => view.model.once('change:subject', resolve));
+
+                    expect(sizzle('.chat-event:last', view.el).pop().textContent.trim()).toBe('Topic set by ralphm');
+                    const head_desc = await u.waitUntil(() => view.el.querySelector('.chat-head__desc'));
+                    expect(head_desc?.textContent.trim()).toBe(text);
+
+                    stanza = u.toStanza(
+                        `<message xmlns="jabber:client" to="jc@opkode.com/_converse.js-60429116" type="groupchat" from="jdev@conference.jabber.org/ralphm">
+                            <subject>This is a message subject</subject>
+                            <body>This is a message</body>
+                        </message>`);
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                    await new Promise(resolve => view.once('messageInserted', resolve));
+                    expect(sizzle('.chat-msg__subject', view.el).length).toBe(1);
+                    expect(sizzle('.chat-msg__subject', view.el).pop().textContent.trim()).toBe('This is a message subject');
+                    expect(sizzle('.chat-msg__text').length).toBe(1);
+                    expect(sizzle('.chat-msg__text').pop().textContent.trim()).toBe('This is a message');
+                    expect(view.el.querySelector('.chat-head__desc').textContent.trim()).toBe(text);
+
+                    // Removes current topic
+                    stanza = u.toStanza(
+                        `<message xmlns="jabber:client" to="jc@opkode.com/_converse.js-60429116" type="groupchat" from="jdev@conference.jabber.org/ralphm">
+                            <subject/>
+                        </message>`);
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                    await new Promise(resolve => view.model.once('change:subject', resolve));
+                    await u.waitUntil(() => view.el.querySelector('.chat-head__desc') === null);
+                    expect(view.el.querySelector('.chat-info:last-child').textContent.trim()).toBe("Topic cleared by ralphm");
+                    done();
+                }));
+
+                it("can be toggled by the user",
+                    mock.initConverse(
+                        ['rosterGroupsFetched'], {},
+                        async function (done, _converse) {
+
+                    await test_utils.openAndEnterChatRoom(_converse, 'jdev@conference.jabber.org', 'jc');
+                    const text = 'Jabber/XMPP Development | RFCs and Extensions: https://xmpp.org/ | Protocol and XSF discussions: xsf@muc.xmpp.org';
+                    let stanza = u.toStanza(`
+                        <message xmlns="jabber:client" to="jc@opkode.com/_converse.js-60429116" type="groupchat" from="jdev@conference.jabber.org/ralphm">
+                            <subject>${text}</subject>
+                            <delay xmlns="urn:xmpp:delay" stamp="2014-02-04T09:35:39Z" from="jdev@conference.jabber.org"/>
+                            <x xmlns="jabber:x:delay" stamp="20140204T09:35:39" from="jdev@conference.jabber.org"/>
+                        </message>`);
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                    const view = _converse.chatboxviews.get('jdev@conference.jabber.org');
+                    await new Promise(resolve => view.model.once('change:subject', resolve));
+
+                    expect(sizzle('.chat-event:last', view.el).pop().textContent.trim()).toBe('Topic set by ralphm');
+                    const head_desc = await u.waitUntil(() => view.el.querySelector('.chat-head__desc'));
+                    expect(head_desc?.textContent.trim()).toBe(text);
+
+                    stanza = u.toStanza(
+                        `<message xmlns="jabber:client" to="jc@opkode.com/_converse.js-60429116" type="groupchat" from="jdev@conference.jabber.org/ralphm">
+                            <subject>This is a message subject</subject>
+                            <body>This is a message</body>
+                        </message>`);
+                    _converse.connection._dataRecv(test_utils.createRequest(stanza));
+                    await new Promise(resolve => view.once('messageInserted', resolve));
+                    expect(sizzle('.chat-msg__subject', view.el).length).toBe(1);
+                    expect(sizzle('.chat-msg__subject', view.el).pop().textContent.trim()).toBe('This is a message subject');
+                    expect(sizzle('.chat-msg__text').length).toBe(1);
+                    expect(sizzle('.chat-msg__text').pop().textContent.trim()).toBe('This is a message');
+                    const topic_el = view.el.querySelector('.chat-head__desc');
+                    expect(topic_el.textContent.trim()).toBe(text);
+                    expect(u.isVisible(topic_el)).toBe(true);
+
+                    const toggle = view.el.querySelector('.hide-topic');
+                    expect(toggle.textContent).toBe('Hide topic');
+                    toggle.click();
+                    await u.waitUntil(() => !u.isVisible(topic_el));
+                    expect(view.el.querySelector('.hide-topic').textContent).toBe('Show topic');
+                    done();
+                }));
+            });
+
+
             it("clears cached messages when it gets closed and clear_messages_on_reconnection is true",
                 mock.initConverse(
                     ['rosterGroupsFetched'], {'clear_messages_on_reconnection': true},
@@ -1930,52 +2024,6 @@
                 }, 500);
             }));
 
-            it("shows the room topic in the header",
-                mock.initConverse(
-                    ['rosterGroupsFetched'], {},
-                    async function (done, _converse) {
-
-                await test_utils.openAndEnterChatRoom(_converse, 'jdev@conference.jabber.org', 'jc');
-                const text = 'Jabber/XMPP Development | RFCs and Extensions: https://xmpp.org/ | Protocol and XSF discussions: xsf@muc.xmpp.org';
-                let stanza = u.toStanza(`
-                    <message xmlns="jabber:client" to="jc@opkode.com/_converse.js-60429116" type="groupchat" from="jdev@conference.jabber.org/ralphm">
-                        <subject>${text}</subject>
-                        <delay xmlns="urn:xmpp:delay" stamp="2014-02-04T09:35:39Z" from="jdev@conference.jabber.org"/>
-                        <x xmlns="jabber:x:delay" stamp="20140204T09:35:39" from="jdev@conference.jabber.org"/>
-                    </message>`);
-                _converse.connection._dataRecv(test_utils.createRequest(stanza));
-                const view = _converse.chatboxviews.get('jdev@conference.jabber.org');
-                await new Promise(resolve => view.model.once('change:subject', resolve));
-
-                expect(sizzle('.chat-event:last', view.el).pop().textContent.trim()).toBe('Topic set by ralphm');
-                const head_desc = await u.waitUntil(() => view.el.querySelector('.chat-head__desc'));
-                expect(head_desc?.textContent.trim()).toBe(text);
-
-                stanza = u.toStanza(
-                    `<message xmlns="jabber:client" to="jc@opkode.com/_converse.js-60429116" type="groupchat" from="jdev@conference.jabber.org/ralphm">
-                         <subject>This is a message subject</subject>
-                         <body>This is a message</body>
-                     </message>`);
-                _converse.connection._dataRecv(test_utils.createRequest(stanza));
-                await new Promise(resolve => view.once('messageInserted', resolve));
-                expect(sizzle('.chat-msg__subject', view.el).length).toBe(1);
-                expect(sizzle('.chat-msg__subject', view.el).pop().textContent.trim()).toBe('This is a message subject');
-                expect(sizzle('.chat-msg__text').length).toBe(1);
-                expect(sizzle('.chat-msg__text').pop().textContent.trim()).toBe('This is a message');
-                expect(view.el.querySelector('.chat-head__desc').textContent.trim()).toBe(text);
-
-                // Removes current topic
-                stanza = u.toStanza(
-                    `<message xmlns="jabber:client" to="jc@opkode.com/_converse.js-60429116" type="groupchat" from="jdev@conference.jabber.org/ralphm">
-                         <subject/>
-                     </message>`);
-                _converse.connection._dataRecv(test_utils.createRequest(stanza));
-                await new Promise(resolve => view.model.once('change:subject', resolve));
-                await u.waitUntil(() => view.el.querySelector('.chat-head__desc') === null);
-                expect(view.el.querySelector('.chat-info:last-child').textContent.trim()).toBe("Topic cleared by ralphm");
-                done();
-            }));
-
             it("reconnects when no-acceptable error is returned when sending a message",
                 mock.initConverse(
                     ['rosterGroupsFetched'], {},
@@ -2438,7 +2486,7 @@
                 expect(view.model.features.get('temporary')).toBe(true);
                 expect(view.model.features.get('unmoderated')).toBe(true);
                 expect(view.model.features.get('unsecured')).toBe(false);
-                expect(view.el.querySelector('.chatbox-title__text').textContent.trim()).toBe('New room name');
+                await u.waitUntil(() => view.el.querySelector('.chatbox-title__text')?.textContent.trim() === 'New room name');
                 done();
             }));
 

spec/xss.js

@@ -236,8 +236,7 @@
                     'author': 'ralphm'
                 }});
                 expect(sizzle('.chat-event:last').pop().textContent.trim()).toBe('Topic set by ralphm');
-                const desc = await u.waitUntil(() => view.el.querySelector('.chat-head__desc'));
-                expect(desc.textContent.trim()).toBe(subject);
+                await u.waitUntil(() => view.el.querySelector('.chat-head__desc')?.textContent.trim() === subject);
                 done();
             }));
         });

src/components/dropdown.js

@@ -8,7 +8,6 @@ import converse from "@converse/headless/converse-core";
 const u = converse.env.utils;
 
 
-
 export class Dropdown extends CustomElement {
 
     static get properties () {

src/converse-bookmark-views.js

@@ -36,9 +36,9 @@ converse.plugins.add('converse-bookmark-views', {
         // plugin architecture they will replace existing methods on the
         // relevant objects or classes.
         ChatRoomView: {
-            async getHeadingButtons () {
+            getHeadingButtons () {
                 const { _converse } = this.__super__;
-                const buttons = await this.__super__.getHeadingButtons.call(this);
+                const buttons = this.__super__.getHeadingButtons.apply(this, arguments);
                 if (_converse.allow_bookmarks) {
                     const supported = _converse.checkBookmarksSupport();
                     const bookmarked = this.model.get('bookmarked');

src/converse-minimize.js

@@ -139,9 +139,9 @@ converse.plugins.add('converse-minimize', {
                 return result;
             },
 
-            async getHeadingButtons () {
+            getHeadingButtons () {
                 const { _converse } = this.__super__;
-                const buttons = await this.__super__.getHeadingButtons.call(this);
+                const buttons = this.__super__.getHeadingButtons.apply(this, arguments);
                 const data = {
                     'a_class': 'toggle-chatbox-button',
                     'handler': ev => this.minimize(ev),

src/converse-muc-views.js

@@ -8,7 +8,7 @@ import "converse-modal";
 import "@converse/headless/utils/muc";
 import { Model } from 'skeletor.js/src/model.js';
 import { View } from 'skeletor.js/src/view.js';
-import { head, isString, isUndefined } from "lodash";
+import { debounce, head, isString, isUndefined } from "lodash";
 import { BootstrapModal } from "./converse-modal.js";
 import { render } from "lit-html";
 import { __ } from '@converse/headless/i18n';
@@ -703,7 +703,10 @@ converse.plugins.add('converse-muc-views', {
                 this.listenTo(this.model.notifications, 'change', this.renderNotifications);
                 this.listenTo(this.model.session, 'change:connection_status', this.onConnectionStatusChanged);
 
-                this.listenTo(this.model, 'change', this.renderHeading);
+                const user_settings = _converse.api.user.settings.getModel();
+                this.listenTo(user_settings, 'change:mucs_with_hidden_subject', this.renderHeading);
+
+                this.listenTo(this.model, 'change', debounce(() => this.renderHeading(), 250));
                 this.listenTo(this.model, 'change:hidden_occupants', this.updateOccupantsToggle);
                 this.listenTo(this.model, 'change:subject', this.setChatRoomSubject);
                 this.listenTo(this.model, 'configurationNeeded', this.getAndRenderConfigurationForm);
@@ -1171,7 +1174,7 @@ converse.plugins.add('converse-muc-views', {
                 }
             },
 
-            async getHeadingButtons () {
+            getHeadingButtons (subject_hidden) {
                 const buttons = [{
                     'i18n_text': __('Details'),
                     'i18n_title': __('Show more information about this groupchat'),
@@ -1227,9 +1230,6 @@ converse.plugins.add('converse-muc-views', {
                     });
                 }
 
-                const muc_jid = this.model.get('jid');
-                const jids = await api.user.settings.get('mucs_with_hidden_subject', [])
-                const subject_hidden = jids.includes(muc_jid);
                 const subject = this.model.get('subject');
                 if (subject && subject.text) {
                     buttons.push({
@@ -1238,7 +1238,7 @@ converse.plugins.add('converse-muc-views', {
                             __('Show the topic message in the heading') :
                             __('Hide the topic in the heading'),
                         'handler': ev => this.toggleTopic(ev),
-                        'a_class': '',
+                        'a_class': 'hide-topic',
                         'icon_class': 'fa-minus-square',
                         'name': 'toggle-topic'
                     });
@@ -1268,12 +1268,15 @@ converse.plugins.add('converse-muc-views', {
              * @method _converse.ChatRoomView#generateHeadingTemplate
              */
             async generateHeadingTemplate () {
-                const heading_btns = await this.getHeadingButtons();
+                const jids = await api.user.settings.get('mucs_with_hidden_subject', [])
+                const subject_hidden = jids.includes(this.model.get('jid'));
+                const heading_btns = this.getHeadingButtons(subject_hidden);
                 const standalone_btns = heading_btns.filter(b => b.standalone);
                 const dropdown_btns = heading_btns.filter(b => !b.standalone);
                 return tpl_chatroom_head(
                     Object.assign(this.model.toJSON(), {
                         _converse,
+                        subject_hidden,
                         'dropdown_btns': dropdown_btns.map(b => this.getHeadingDropdownItem(b)),
                         'standalone_btns': standalone_btns.map(b => this.getHeadingStandaloneButton(b)),
                         'title': this.model.getDisplayName(),

src/headless/converse-core.js

@@ -529,6 +529,16 @@ const api = _converse.api = {
          * @memberOf _converse.api.user
          */
         settings: {
+            /**
+             * Returns the user settings model. Useful when you want to listen for change events.
+             * @method _converse.api.user.settings.getModel
+             * @returns {Model}
+             * @example _converse.api.user.settings.getModel
+             */
+            getModel () {
+                return user_settings;
+            },
+
             /**
              * Get the value of a particular user setting.
              * @method _converse.api.user.settings.get
@@ -558,8 +568,13 @@ const api = _converse.api = {
              */
             async set (key, val) {
                 await initUserSettings();
-                const o = isObject(key) ? key : {key: val};
-                return user_settings.save(o, {'promise': true});
+                if (isObject(key)) {
+                    return user_settings.save(key, {'promise': true});
+                } else {
+                    const o = {};
+                    o[key] = val;
+                    return user_settings.save(o, {'promise': true});
+                }
             }
         }
     },
@@ -1086,6 +1101,10 @@ function clearSession  () {
         _converse.session.destroy();
         delete _converse.session;
     }
+    if (_converse.shouldClearCache()) {
+        const model = _converse.api.user.settings.getModel();
+        model.clear();
+    }
     /**
      * Synchronouse event triggered once the user session has been cleared,
      * for example when the user has logged out or when Converse has