Also check for duplicates in one-on-one messages

dist/converse.js

@@ -61643,6 +61643,39 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         return false;
       },
 
+      findDuplicateFromOriginID(stanza) {
+        const origin_id = sizzle(`origin-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
+
+        if (!origin_id) {
+          return false;
+        }
+
+        return this.messages.findWhere({
+          'origin_id': origin_id.getAttribute('id'),
+          'sender': 'me'
+        });
+      },
+
+      async hasDuplicateStanzaID(stanza) {
+        const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
+
+        if (!stanza_id) {
+          return false;
+        }
+
+        const by_jid = stanza_id.getAttribute('by');
+        const result = await _converse.api.disco.supports(Strophe.NS.SID, by_jid);
+
+        if (!result.length) {
+          return false;
+        }
+
+        const query = {};
+        query[`stanza_id ${by_jid}`] = stanza_id.getAttribute('id');
+        const msg = this.messages.findWhere(query);
+        return !_.isNil(msg);
+      },
+
       sendMarker(to_jid, id, type) {
         const stanza = $msg({
           'from': _converse.connection.jid,
@@ -62251,7 +62284,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         },
               chatbox = this.getChatBox(contact_jid, chatbox_attrs, has_body);
 
-        if (chatbox && !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt(stanza, from_jid, is_carbon, is_me) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
+        if (chatbox && !chatbox.findDuplicateFromOriginID(stanza) && !(await chatbox.hasDuplicateStanzaID(stanza)) && !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt(stanza, from_jid, is_carbon, is_me) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
           const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza);
 
           if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) {
@@ -66935,26 +66968,6 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
         return data;
       },
 
-      async isDuplicate(message, original_stanza) {
-        const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, message).pop();
-
-        if (!stanza_id) {
-          return false;
-        }
-
-        const by_jid = stanza_id.getAttribute('by');
-        const result = await _converse.api.disco.supports(Strophe.NS.SID, by_jid);
-
-        if (!result.length) {
-          return false;
-        }
-
-        const query = {};
-        query[`stanza_id ${by_jid}`] = stanza_id.getAttribute('id');
-        const msg = this.messages.findWhere(query);
-        return !_.isNil(msg);
-      },
-
       fetchFeaturesIfConfigurationChanged(stanza) {
         const configuration_changed = stanza.querySelector("status[code='104']"),
               logging_enabled = stanza.querySelector("status[code='170']"),
@@ -66978,7 +66991,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
                      acknowledged[xmlns="${Strophe.NS.MARKERS}"]`, stanza).length > 0;
       },
 
-      reflectionHandled(stanza) {
+      handleReflection(stanza) {
         /* Handle a MUC reflected message and return true if so.
          *
          * Parameters:
@@ -66988,16 +67001,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
         const own_message = Strophe.getResourceFromJid(from) == this.get('nick');
 
         if (own_message) {
-          const origin_id = sizzle(`origin-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
-
-          if (!origin_id) {
-            return false;
-          }
-
-          const msg = this.messages.findWhere({
-            'origin_id': origin_id.getAttribute('id'),
-            'sender': 'me'
-          });
+          const msg = this.findDuplicateFromOriginID(stanza);
 
           if (msg) {
             const attrs = {};
@@ -67070,17 +67074,15 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
           stanza = forwarded.querySelector('message');
         }
 
-        if (await this.isDuplicate(stanza, original_stanza)) {
-          return;
+        if (this.handleReflection(stanza) || (await this.hasDuplicateStanzaID(stanza)) || this.handleMessageCorrection(stanza) || this.isReceipt(stanza) || this.isChatMarker(stanza)) {
+          return _converse.emit('message', {
+            'stanza': original_stanza
+          });
         }
 
         const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza);
 
-        if (!attrs.nick) {
-          return;
-        }
-
-        if (!this.handleMessageCorrection(stanza) && !this.isReceipt(stanza) && !this.isChatMarker(stanza) && !this.reflectionHandled(stanza) && !this.subjectChangeHandled(attrs) && !this.ignorableCSN(attrs) && (attrs['chat_state'] || !_utils_form__WEBPACK_IMPORTED_MODULE_7__["default"].isEmptyMessage(attrs))) {
+        if (attrs.nick && !this.subjectChangeHandled(attrs) && !this.ignorableCSN(attrs) && (attrs['chat_state'] || !_utils_form__WEBPACK_IMPORTED_MODULE_7__["default"].isEmptyMessage(attrs))) {
           const msg = this.messages.create(attrs);
           this.incrementUnreadMsgCounter(msg);
 
@@ -67091,13 +67093,10 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
           }
         }
 
-        if (attrs.nick !== this.get('nick')) {
-          // We only emit an event if it's not our own message
-          _converse.emit('message', {
-            'stanza': original_stanza,
-            'chatbox': this
-          });
-        }
+        _converse.emit('message', {
+          'stanza': original_stanza,
+          'chatbox': this
+        });
       },
 
       onPresence(pres) {

spec/mam.js

@@ -30,12 +30,6 @@
                     </message>`);
                 _converse.connection._dataRecv(test_utils.createRequest(stanza));
                 await test_utils.waitUntil(() => view.content.querySelectorAll('.chat-msg').length);
-                // XXX: we wait here until the first message appears before
-                // sending the duplicate. If we don't do that, then the
-                // duplicate appears before the promise for `createMessage`
-                // has been resolved, which means that the `isDuplicate`
-                // check fails because the first message doesn't exist yet.
-                //
                 // Not sure whether such a race-condition might pose a problem
                 // in "real-world" situations.
                 stanza = u.toStanza(
@@ -50,9 +44,9 @@
                         </result>
                     </message>`);
 
-                spyOn(view.model, 'isDuplicate').and.callThrough();
+                spyOn(view.model, 'hasDuplicateStanzaID').and.callThrough();
                 view.model.onMessage(stanza);
-                await test_utils.waitUntil(() => view.model.isDuplicate.calls.count());
+                await test_utils.waitUntil(() => view.model.hasDuplicateStanzaID.calls.count());
                 expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
                 done();
             }));
@@ -93,10 +87,10 @@
                         </result>
                     </message>`);
 
-                spyOn(view.model, 'isDuplicate').and.callThrough();
+                spyOn(view.model, 'hasDuplicateStanzaID').and.callThrough();
                 view.model.onMessage(stanza);
-                await test_utils.waitUntil(() => view.model.isDuplicate.calls.count());
-                expect(view.model.isDuplicate.calls.count()).toBe(1);
+                await test_utils.waitUntil(() => view.model.hasDuplicateStanzaID.calls.count());
+                expect(view.model.hasDuplicateStanzaID.calls.count()).toBe(1);
                 expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
 
                 stanza = u.toStanza(
@@ -114,7 +108,7 @@
                         </result>
                     </message>`);
                 view.model.onMessage(stanza);
-                expect(view.model.isDuplicate.calls.count()).toBe(2);
+                expect(view.model.hasDuplicateStanzaID.calls.count()).toBe(2);
                 expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
                 done();
             }))

spec/messages.js

@@ -1215,7 +1215,9 @@
                 }).c('body').t('Message!').up()
                 .c('request', {'xmlns': Strophe.NS.RECEIPTS}).tree();
             await _converse.chatboxes.onMessage(msg);
-            const receipt = sizzle(`received[xmlns="${Strophe.NS.RECEIPTS}"]`, sent_stanzas[1].tree()).pop();
+            const sent_messages = sent_stanzas.map(s => _.isElement(s) ? s : s.nodeTree).filter(s => s.nodeName === 'message');
+            expect(sent_messages.length).toBe(1);
+            const receipt = sizzle(`received[xmlns="${Strophe.NS.RECEIPTS}"]`, sent_messages[0]).pop();
             expect(Strophe.serialize(receipt)).toBe(`<received id="${msg_id}" xmlns="${Strophe.NS.RECEIPTS}"/>`);
             done();
         }));
@@ -2106,6 +2108,8 @@
                 async function (done, _converse) {
 
             test_utils.createContacts(_converse, 'current', 1);
+            await test_utils.waitUntilDiscoConfirmed(_converse, _converse.bare_jid, [], [Strophe.NS.SID]);
+
             const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@localhost';
             await test_utils.openChatBoxFor(_converse, contact_jid);
             const view = await _converse.api.chatviews.get(contact_jid);
@@ -2210,7 +2214,7 @@
 
             await test_utils.openAndEnterChatRoom(_converse, 'room', 'muc.example.com', 'dummy');
             const view = _converse.chatboxviews.get('room@muc.example.com');
-            spyOn(view.model, 'isDuplicate').and.callThrough();
+            spyOn(view.model, 'hasDuplicateStanzaID').and.callThrough();
             let stanza = u.toStanza(`
                 <message xmlns="jabber:client"
                          from="room@muc.example.com/some1"
@@ -2225,8 +2229,8 @@
             _converse.connection._dataRecv(test_utils.createRequest(stanza));
             await test_utils.waitUntil(() => _converse.api.chats.get().length);
             await test_utils.waitUntil(() => view.model.messages.length === 1);
-            await test_utils.waitUntil(() => view.model.isDuplicate.calls.count() === 1);
-            let result = await view.model.isDuplicate.calls.all()[0].returnValue;
+            await test_utils.waitUntil(() => view.model.hasDuplicateStanzaID.calls.count() === 1);
+            let result = await view.model.hasDuplicateStanzaID.calls.all()[0].returnValue;
             expect(result).toBe(false);
 
             stanza = u.toStanza(`
@@ -2241,8 +2245,8 @@
                     <origin-id xmlns="urn:xmpp:sid:0" id="de305d54-75b4-431b-adb2-eb6b9e546013"/>
                 </message>`);
             _converse.connection._dataRecv(test_utils.createRequest(stanza));
-            await test_utils.waitUntil(() => view.model.isDuplicate.calls.count() === 2);
-            result = await view.model.isDuplicate.calls.all()[1].returnValue;
+            await test_utils.waitUntil(() => view.model.hasDuplicateStanzaID.calls.count() === 2);
+            result = await view.model.hasDuplicateStanzaID.calls.all()[1].returnValue;
             expect(result).toBe(true);
             expect(view.model.messages.length).toBe(1);
             done();
@@ -2506,9 +2510,9 @@
                                by="room@muc.example.com"/>
                     <origin-id xmlns="urn:xmpp:sid:0" id="${attrs.origin_id}"/>
                 </message>`);
-            spyOn(view.model, 'reflectionHandled').and.callThrough();
+            spyOn(view.model, 'handleReflection').and.callThrough();
             _converse.connection._dataRecv(test_utils.createRequest(stanza));
-            await test_utils.waitUntil(() => view.model.reflectionHandled.calls.count() === 1);
+            await test_utils.waitUntil(() => view.model.handleReflection.calls.count() === 1);
             expect(view.model.messages.length).toBe(1);
             expect(view.model.messages.at(0).get('stanza_id room@muc.example.com')).toBe("5f3dbc5e-e1d3-4077-a492-693f3769c7ad");
             expect(view.model.messages.at(0).get('origin_id')).toBe(attrs.origin_id);

spec/muc.js

@@ -4200,7 +4200,7 @@
                         // Check that new messages appear under the chat state
                         // notifications
                         msg = $msg({
-                            from: 'lounge@localhost/some1',
+                            from: `${room_jid}/some1`,
                             id: (new Date()).getTime(),
                             to: 'dummy@localhost',
                             type: 'groupchat'

src/headless/converse-chatboxes.js

@@ -313,6 +313,34 @@ converse.plugins.add('converse-chatboxes', {
                 }
                 return false;
             },
+
+            findDuplicateFromOriginID  (stanza) {
+                const origin_id = sizzle(`origin-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
+                if (!origin_id) {
+                    return false;
+                }
+                return this.messages.findWhere({
+                    'origin_id': origin_id.getAttribute('id'),
+                    'sender': 'me'
+                });
+            },
+
+            async hasDuplicateStanzaID (stanza) {
+                const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
+                if (!stanza_id) {
+                    return false;
+                }
+                const by_jid = stanza_id.getAttribute('by');
+                const result = await _converse.api.disco.supports(Strophe.NS.SID, by_jid);
+                if (!result.length) {
+                    return false;
+                }
+                const query = {};
+                query[`stanza_id ${by_jid}`] = stanza_id.getAttribute('id');
+                const msg = this.messages.findWhere(query);
+                return !_.isNil(msg);
+            },
+
             
             sendMarker(to_jid, id, type) {
                 const stanza = $msg({
@@ -821,6 +849,7 @@ converse.plugins.add('converse-chatboxes', {
                     from_jid = stanza.getAttribute('from');
                     to_jid = stanza.getAttribute('to');
                 }
+
                 const from_bare_jid = Strophe.getBareJidFromJid(from_jid),
                       from_resource = Strophe.getResourceFromJid(from_jid),
                       is_me = from_bare_jid === _converse.bare_jid;
@@ -850,6 +879,8 @@ converse.plugins.add('converse-chatboxes', {
                       chatbox = this.getChatBox(contact_jid, chatbox_attrs, has_body);
 
                 if (chatbox &&
+                        !chatbox.findDuplicateFromOriginID(stanza) &&
+                        !await chatbox.hasDuplicateStanzaID(stanza) &&
                         !chatbox.handleMessageCorrection(stanza) &&
                         !chatbox.handleReceipt (stanza, from_jid, is_carbon, is_me) &&
                         !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {

src/headless/converse-muc.js

@@ -948,22 +948,6 @@ converse.plugins.add('converse-muc', {
                 return data;
             },
 
-            async isDuplicate (message, original_stanza) {
-                const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, message).pop();
-                if (!stanza_id) {
-                    return false;
-                }
-                const by_jid = stanza_id.getAttribute('by');
-                const result = await _converse.api.disco.supports(Strophe.NS.SID, by_jid);
-                if (!result.length) {
-                    return false;
-                }
-                const query = {};
-                query[`stanza_id ${by_jid}`] = stanza_id.getAttribute('id');
-                const msg = this.messages.findWhere(query);
-                return !_.isNil(msg);
-            },
-
             fetchFeaturesIfConfigurationChanged (stanza) {
                 const configuration_changed = stanza.querySelector("status[code='104']"),
                       logging_enabled = stanza.querySelector("status[code='170']"),
@@ -989,7 +973,7 @@ converse.plugins.add('converse-muc', {
                      acknowledged[xmlns="${Strophe.NS.MARKERS}"]`, stanza).length > 0;
             },
 
-            reflectionHandled (stanza) {
+            handleReflection (stanza) {
                 /* Handle a MUC reflected message and return true if so.
                  *
                  * Parameters:
@@ -998,14 +982,7 @@ converse.plugins.add('converse-muc', {
                 const from = stanza.getAttribute('from');
                 const own_message = Strophe.getResourceFromJid(from) == this.get('nick');
                 if (own_message) {
-                    const origin_id = sizzle(`origin-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
-                    if (!origin_id) {
-                        return false;
-                    }
-                    const msg = this.messages.findWhere({
-                        'origin_id': origin_id.getAttribute('id'),
-                        'sender': 'me'
-                    });
+                    const msg = this.findDuplicateFromOriginID(stanza);
                     if (msg) {
                         const attrs = {};
                         const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
@@ -1066,20 +1043,18 @@ converse.plugins.add('converse-muc', {
                 if (forwarded) {
                     stanza = forwarded.querySelector('message');
                 }
-                if (await this.isDuplicate(stanza, original_stanza)) {
-                    return;
+                if (this.handleReflection(stanza) ||
+                        await this.hasDuplicateStanzaID(stanza) ||
+                        this.handleMessageCorrection(stanza) ||
+                        this.isReceipt(stanza) ||
+                        this.isChatMarker(stanza)) {
+                    return _converse.emit('message', {'stanza': original_stanza});
                 }
                 const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza);
-                if (!attrs.nick) {
-                    return;
-                }
-                if (!this.handleMessageCorrection(stanza) &&
-                    !this.isReceipt(stanza) &&
-                    !this.isChatMarker(stanza) &&
-                    !this.reflectionHandled(stanza) &&
-                    !this.subjectChangeHandled(attrs) &&
-                    !this.ignorableCSN(attrs) &&
-                    (attrs['chat_state'] || !u.isEmptyMessage(attrs))) {
+                if (attrs.nick &&
+                        !this.subjectChangeHandled(attrs) &&
+                        !this.ignorableCSN(attrs) &&
+                        (attrs['chat_state'] || !u.isEmptyMessage(attrs))) {
 
                     const msg = this.messages.create(attrs);
                     this.incrementUnreadMsgCounter(msg);
@@ -1087,10 +1062,7 @@ converse.plugins.add('converse-muc', {
                         msg.save({'received': moment().format()});
                     }
                 }
-                if (attrs.nick !== this.get('nick')) {
-                    // We only emit an event if it's not our own message
-                    _converse.emit('message', {'stanza': original_stanza, 'chatbox': this});
-                }
+                _converse.emit('message', {'stanza': original_stanza, 'chatbox': this});
             },
 
             onPresence (pres) {