MUC: only allow corrections with a matching sender `occupant-id`

Also, don't render messages from a sender with a different `occupant-id`
as a followup message.

src/headless/plugins/chat/message.js

@@ -143,12 +143,11 @@ const MessageMixin = {
         }
         const date = dayjs(this.get('time'));
         return this.get('from') === prev_model.get('from') &&
-            !this.isMeCommand() &&
-            !prev_model.isMeCommand() &&
-            this.get('type') !== 'info' &&
-            prev_model.get('type') !== 'info' &&
+            !this.isMeCommand() && !prev_model.isMeCommand() &&
+            !!this.get('is_encrypted') === !!prev_model.get('is_encrypted') &&
+            this.get('type') === prev_model.get('type') && this.get('type') !== 'info' &&
             date.isBefore(dayjs(prev_model.get('time')).add(10, 'minutes')) &&
-            !!this.get('is_encrypted') === !!prev_model.get('is_encrypted');
+            (this.get('type') === 'groupchat' ? this.get('occupant_id') === prev_model.get('occupant_id') : true);
     },
 
     getDisplayName () {

src/headless/plugins/chat/model.js

@@ -6,7 +6,7 @@ import log from '@converse/headless/log';
 import pick from "lodash-es/pick";
 import { Model } from '@converse/skeletor/src/model.js';
 import { _converse, api, converse } from "../../core.js";
-import { debouncedPruneHistory } from '@converse/headless/shared/chat/utils.js';
+import { debouncedPruneHistory, handleCorrection } from '@converse/headless/shared/chat/utils.js';
 import { getMediaURLsMetadata } from '@converse/headless/shared/parsers.js';
 import { getOpenPromise } from '@converse/openpromise';
 import { initStorage } from '@converse/headless/utils/storage.js';
@@ -234,7 +234,7 @@ const ChatBox = ModelWithContact.extend({
                 this.notifications.set('chat_state', attrs.chat_state);
             }
             if (u.shouldCreateMessage(attrs)) {
-                const msg = this.handleCorrection(attrs) || await this.createMessage(attrs);
+                const msg = handleCorrection(this, attrs) || await this.createMessage(attrs);
                 this.notifications.set({'chat_state': null});
                 this.handleUnreadMessage(msg);
             }
@@ -605,47 +605,6 @@ const ChatBox = ModelWithContact.extend({
         return false;
     },
 
-    /**
-     * Determines whether the passed in message attributes represent a
-     * message which corrects a previously received message, or an
-     * older message which has already been corrected.
-     * In both cases, update the corrected message accordingly.
-     * @private
-     * @method _converse.ChatBox#handleCorrection
-     * @param { object } attrs - Attributes representing a received
-     *  message, as returned by {@link parseMessage}
-     * @returns { _converse.Message|undefined } Returns the corrected
-     *  message or `undefined` if not applicable.
-     */
-    handleCorrection (attrs) {
-        if (!attrs.replace_id || !attrs.from) {
-            return;
-        }
-        const message = this.messages.findWhere({'msgid': attrs.replace_id, 'from': attrs.from});
-        if (!message) {
-            return;
-        }
-        const older_versions = message.get('older_versions') || {};
-        if ((attrs.time < message.get('time')) && message.get('edited')) {
-            // This is an older message which has been corrected afterwards
-            older_versions[attrs.time] = attrs['message'];
-            message.save({'older_versions': older_versions});
-        } else {
-            // This is a correction of an earlier message we already received
-            if (Object.keys(older_versions).length) {
-                older_versions[message.get('edited')] = message.getMessageText();
-            } else {
-                older_versions[message.get('time')] = message.getMessageText();
-            }
-            attrs = Object.assign(attrs, { older_versions });
-            delete attrs['msgid']; // We want to keep the msgid of the original message
-            delete attrs['id']; // Delete id, otherwise a new cache entry gets created
-            attrs['time'] = message.get('time');
-            message.save(attrs);
-        }
-        return message;
-    },
-
     /**
      * Returns an already cached message (if it exists) based on the
      * passed in attributes map.

src/headless/plugins/muc/muc.js

@@ -11,12 +11,13 @@ import { Model } from '@converse/skeletor/src/model.js';
 import { Strophe, $build, $iq, $msg, $pres } from 'strophe.js/src/strophe';
 import { _converse, api, converse } from '../../core.js';
 import { computeAffiliationsDelta, setAffiliations, getAffiliationList }  from './affiliations/utils.js';
+import { handleCorrection } from '@converse/headless/shared/chat/utils.js';
 import { getOpenPromise } from '@converse/openpromise';
 import { initStorage } from '@converse/headless/utils/storage.js';
-import { isArchived, getMediaURLsMetadata } from '@converse/headless/shared/parsers';
+import { isArchived, getMediaURLsMetadata } from '@converse/headless/shared/parsers.js';
 import { isUniView, getUniqueId, safeSave } from '@converse/headless/utils/core.js';
 import { parseMUCMessage, parseMUCPresence } from './parsers.js';
-import { sendMarker } from '@converse/headless/shared/actions';
+import { sendMarker } from '@converse/headless/shared/actions.js';
 
 const OWNER_COMMANDS = ['owner'];
 const ADMIN_COMMANDS = ['admin', 'ban', 'deop', 'destroy', 'member', 'op', 'revoke'];
@@ -2267,7 +2268,7 @@ const ChatRoomMixin = {
             this.updateNotifications(attrs.nick, attrs.chat_state);
         }
         if (u.shouldCreateGroupchatMessage(attrs)) {
-            const msg = this.handleCorrection(attrs) || (await this.createMessage(attrs));
+            const msg = handleCorrection(this, attrs) || (await this.createMessage(attrs));
             this.removeNotification(attrs.nick, ['composing', 'paused']);
             this.handleUnreadMessage(msg);
         }

src/headless/shared/chat/utils.js

@@ -58,4 +58,55 @@ export function getMediaURLs (arr, text, offset=0) {
     }).filter(o => o);
 }
 
+
+/**
+ * Determines whether the passed in message attributes represent a
+ * message which corrects a previously received message, or an
+ * older message which has already been corrected.
+ * In both cases, update the corrected message accordingly.
+ * @private
+ * @method _converse.ChatBox#handleCorrection
+ * @param { _converse.ChatBox | _converse.ChatRoom }
+ * @param { object } attrs - Attributes representing a received
+ *  message, as returned by {@link parseMessage}
+ * @returns { _converse.Message|undefined } Returns the corrected
+ *  message or `undefined` if not applicable.
+ */
+export function handleCorrection (model, attrs) {
+    if (!attrs.replace_id || !attrs.from) {
+        return;
+    }
+
+    const query = (attrs.type === 'groupchat' && attrs.occupant_id)
+        ? ({ attributes: m }) => m.msgid === attrs.replace_id && m.occupant_id == attrs.occupant_id
+        // eslint-disable-next-line no-eq-null
+        : ({ attributes: m }) => m.msgid === attrs.replace_id && m.from === attrs.from && m.occupant_id == null
+
+    const message = model.messages.models.find(query);
+    if (!message) {
+        return;
+    }
+
+    const older_versions = message.get('older_versions') || {};
+    if ((attrs.time < message.get('time')) && message.get('edited')) {
+        // This is an older message which has been corrected afterwards
+        older_versions[attrs.time] = attrs['message'];
+        message.save({'older_versions': older_versions});
+    } else {
+        // This is a correction of an earlier message we already received
+        if (Object.keys(older_versions).length) {
+            older_versions[message.get('edited')] = message.getMessageText();
+        } else {
+            older_versions[message.get('time')] = message.getMessageText();
+        }
+        attrs = Object.assign(attrs, { older_versions });
+        delete attrs['msgid']; // We want to keep the msgid of the original message
+        delete attrs['id']; // Delete id, otherwise a new cache entry gets created
+        attrs['time'] = message.get('time');
+        message.save(attrs);
+    }
+    return message;
+}
+
+
 export const debouncedPruneHistory = debounce(pruneHistory, 500);

src/headless/utils/core.js

@@ -16,6 +16,7 @@ import { Strophe } from 'strophe.js/src/strophe.js';
 import { getOpenPromise } from '@converse/openpromise';
 import { setUserJID, } from '@converse/headless/utils/init.js';
 import { settings_api } from '@converse/headless/shared/settings/api.js';
+import { stanza, toStanza } from './stanza.js';
 
 export function isEmptyMessage (attrs) {
     if (attrs instanceof Model) {
@@ -50,6 +51,21 @@ export async function tearDown () {
     return _converse;
 }
 
+/**
+ * Given a message object, return its text with @ chars
+ * inserted before the mentioned nicknames.
+ */
+export function prefixMentions (message) {
+    let text = message.getMessageText();
+    (message.get('references') || [])
+        .sort((a, b) => b.begin - a.begin)
+        .forEach(ref => {
+            text = `${text.slice(0, ref.begin)}@${text.slice(ref.begin)}`
+        });
+    return text;
+}
+
+
 
 /**
  * The utils object
@@ -57,7 +73,6 @@ export async function tearDown () {
  */
 const u = {};
 
-
 u.isTagEqual = function (stanza, name) {
     if (stanza.nodeTree) {
         return u.isTagEqual(stanza.nodeTree, name);
@@ -70,9 +85,6 @@ u.isTagEqual = function (stanza, name) {
     }
 }
 
-const parser = new DOMParser();
-const parserErrorNS = parser.parseFromString('invalid', 'text/xml')
-                            .getElementsByTagName("parsererror")[0].namespaceURI;
 
 u.getJIDFromURI = function (jid) {
     return jid.startsWith('xmpp:') && jid.endsWith('?join')
@@ -80,14 +92,6 @@ u.getJIDFromURI = function (jid) {
         : jid;
 }
 
-u.toStanza = function (string) {
-    const node = parser.parseFromString(string, "text/xml");
-    if (node.getElementsByTagNameNS(parserErrorNS, 'parsererror').length) {
-        throw new Error(`Parser Error: ${string}`);
-    }
-    return node.firstElementChild;
-}
-
 u.getLongestSubstring = function (string, candidates) {
     function reducer (accumulator, current_value) {
         if (string.startsWith(current_value)) {
@@ -103,20 +107,6 @@ u.getLongestSubstring = function (string, candidates) {
     return candidates.reduce(reducer, '');
 }
 
-/**
- * Given a message object, return its text with @ chars
- * inserted before the mentioned nicknames.
- */
-export function prefixMentions (message) {
-    let text = message.getMessageText();
-    (message.get('references') || [])
-        .sort((a, b) => b.begin - a.begin)
-        .forEach(ref => {
-            text = `${text.slice(0, ref.begin)}@${text.slice(ref.begin)}`
-        });
-    return text;
-}
-
 u.isValidJID = function (jid) {
     if (typeof jid === 'string') {
         return compact(jid.split('@')).length === 2 && !jid.startsWith('@') && !jid.endsWith('@');
@@ -585,5 +575,7 @@ export function decodeHTMLEntities (str) {
 export default Object.assign({
     prefixMentions,
     isEmptyMessage,
-    getUniqueId
+    getUniqueId,
+    toStanza,
+    stanza,
 }, u);

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

@@ -1,6 +1,6 @@
 /*global mock, converse */
 
-const { $msg, $pres, Strophe, u } = converse.env;
+const { $msg, $pres, Strophe, u, stanza } = converse.env;
 
 describe("A Groupchat Message", function () {
 
@@ -8,8 +8,7 @@ describe("A Groupchat Message", function () {
             mock.initConverse([], {}, async function (_converse) {
 
         const muc_jid = 'lounge@montague.lit';
-        await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
-        const view = _converse.chatboxviews.get(muc_jid);
+        const model = await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
         const stanza = $pres({
                 to: 'romeo@montague.lit/_converse.js-29092160',
                 from: 'coven@chat.shakespeare.lit/newguy'
@@ -22,13 +21,14 @@ describe("A Groupchat Message", function () {
             }).tree();
         _converse.connection._dataRecv(mock.createRequest(stanza));
         const msg_id = u.getUniqueId();
-        await view.model.handleMessageStanza($msg({
+        await model.handleMessageStanza($msg({
                 'from': 'lounge@montague.lit/newguy',
                 'to': _converse.connection.jid,
                 'type': 'groupchat',
                 'id': msg_id,
             }).c('body').t('But soft, what light through yonder airlock breaks?').tree());
 
+        const view = _converse.chatboxviews.get(muc_jid);
         await u.waitUntil(() => view.querySelectorAll('.chat-msg').length);
         expect(view.querySelectorAll('.chat-msg').length).toBe(1);
         expect(view.querySelector('.chat-msg__text').textContent)
@@ -262,3 +262,80 @@ describe("A Groupchat Message", function () {
         await u.waitUntil(() => !u.hasClass('correcting', view.querySelector('.chat-msg')), 500);
     }));
 });
+
+
+describe('A Groupchat Message XEP-0308 correction ', function () {
+    it(
+        "is ignored if it's from a different occupant-id",
+        mock.initConverse([], {}, async function (_converse) {
+            const muc_jid = 'lounge@montague.lit';
+            const features = [...mock.default_muc_features, Strophe.NS.OCCUPANTID];
+            const model = await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo', features);
+
+            const msg_id = u.getUniqueId();
+            await model.handleMessageStanza(
+                stanza`
+                <message
+                    from="lounge@montague.lit/newguy"
+                    to="_converse.connection.jid"
+                    type="groupchat"
+                    id="${msg_id}">
+
+                    <body>But soft, what light through yonder airlock breaks?</body>
+                    <occupant-id xmlns="urn:xmpp:occupant-id:0" id="1"></occupant-id>
+                </message>`
+            );
+
+            const view = _converse.chatboxviews.get(muc_jid);
+            await u.waitUntil(() => view.querySelectorAll('.chat-msg').length);
+            expect(model.messages.at(0).get('body')).toBe('But soft, what light through yonder airlock breaks?');
+
+            await model.handleMessageStanza(
+                stanza`
+                <message
+                    from="lounge@montague.lit/newguy"
+                    to="_converse.connection.jid"
+                    type="groupchat"
+                    id="${msg_id}">
+
+                    <body>But soft, what light through yonder chimney breaks?</body>
+                    <occupant-id xmlns="urn:xmpp:occupant-id:0" id="2"></occupant-id>
+                </message>`
+            );
+
+            await u.waitUntil(() => view.querySelectorAll('.chat-msg').length === 2);
+            expect(model.messages.length).toBe(2);
+            expect(model.messages.at(0).get('body')).toBe('But soft, what light through yonder airlock breaks?');
+            expect(model.messages.at(0).get('edited')).toBeFalsy();
+
+            expect(model.messages.at(1).get('body')).toBe('But soft, what light through yonder chimney breaks?');
+            expect(model.messages.at(1).get('edited')).toBeFalsy();
+
+            await model.handleMessageStanza(
+                stanza`
+                <message
+                    from="lounge@montague.lit/newguy"
+                    to="_converse.connection.jid"
+                    type="groupchat"
+                    id="${msg_id}">
+
+                    <body>But soft, what light through yonder hatch breaks?</body>
+                </message>`
+            );
+
+            await u.waitUntil(() => view.querySelectorAll('.chat-msg').length === 3);
+            expect(model.messages.length).toBe(3);
+            expect(model.messages.at(0).get('body')).toBe('But soft, what light through yonder airlock breaks?');
+            expect(model.messages.at(0).get('edited')).toBeFalsy();
+
+            expect(model.messages.at(1).get('body')).toBe('But soft, what light through yonder chimney breaks?');
+            expect(model.messages.at(1).get('edited')).toBeFalsy();
+
+            expect(model.messages.at(2).get('body')).toBe('But soft, what light through yonder hatch breaks?');
+            expect(model.messages.at(2).get('edited')).toBeFalsy();
+
+            const message_els = Array.from(view.querySelectorAll('.chat-msg'));
+            expect(message_els.reduce((acc, m) => acc && u.hasClass('chat-msg--followup', m), true)).toBe(false);
+        })
+    );
+});