Fixes #1353 Don't expect delivery receipts to have type `chat`

CHANGES.md

@@ -4,6 +4,7 @@
 
 - Bugfix: MUC commands were being ignored
 - UI: Always show the OMEMO lock icon (grayed out if not available).
+- #1353 Message Delivery Receipts not working because of the message "type" attribute
 - #1374 Can't load embedded chat when changing `view_mode` between page reloads
 - #1376 Fixed some alignment issues in the sidebar
 - #1378 Message Delivery Receipts were being sent for carbons and own messages

dist/converse.js

@@ -54367,7 +54367,7 @@ _converse_headless_converse_core__WEBPACK_IMPORTED_MODULE_3__["default"].plugins
             break;
 
           case 'help':
-            this.showHelpMessages([`<strong>/admin</strong>: ${__("Change user's affiliation to admin")}`, `<strong>/ban</strong>: ${__('Ban user from groupchat')}`, `<strong>/clear</strong>: ${__('Remove messages')}`, `<strong>/deop</strong>: ${__('Change user role to participant')}`, `<strong>/help</strong>: ${__('Show this menu')}`, `<strong>/kick</strong>: ${__('Kick user from groupchat')}`, `<strong>/me</strong>: ${__('Write in 3rd person')}`, `<strong>/member</strong>: ${__('Grant membership to a user')}`, `<strong>/mute</strong>: ${__("Remove user's ability to post messages")}`, `<strong>/nick</strong>: ${__('Change your nickname')}`, `<strong>/op</strong>: ${__('Grant moderator role to user')}`, `<strong>/owner</strong>: ${__('Grant ownership of this groupchat')}`, `<strong>/register</strong>: ${__("Register a nickname for this room")}`, `<strong>/revoke</strong>: ${__("Revoke user's membership")}`, `<strong>/subject</strong>: ${__('Set groupchat subject')}`, `<strong>/topic</strong>: ${__('Set groupchat subject (alias for /subject)')}`, `<strong>/voice</strong>: ${__('Allow muted user to post messages')}`]);
+            this.showHelpMessages([`<strong>/admin</strong>: ${__("Change user's affiliation to admin")}`, `<strong>/ban</strong>: ${__('Ban user from groupchat')}`, `<strong>/clear</strong>: ${__('Remove messages')}`, `<strong>/deop</strong>: ${__('Change user role to participant')}`, `<strong>/help</strong>: ${__('Show this menu')}`, `<strong>/kick</strong>: ${__('Kick user from groupchat')}`, `<strong>/me</strong>: ${__('Write in 3rd person')}`, `<strong>/member</strong>: ${__('Grant membership to a user')}`, `<strong>/mute</strong>: ${__("Remove user's ability to post messages")}`, `<strong>/nick</strong>: ${__('Change your nickname')}`, `<strong>/op</strong>: ${__('Grant moderator role to user')}`, `<strong>/owner</strong>: ${__('Grant ownership of this groupchat')}`, `<strong>/register</strong>: ${__("Register a nickname for this groupchat")}`, `<strong>/revoke</strong>: ${__("Revoke user's membership")}`, `<strong>/subject</strong>: ${__('Set groupchat subject')}`, `<strong>/topic</strong>: ${__('Set groupchat subject (alias for /subject)')}`, `<strong>/voice</strong>: ${__('Allow muted user to post messages')}`]);
             break;
 
           case 'kick':
@@ -61415,7 +61415,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
       'send_chat_state_notifications': true
     });
 
-    _converse.api.promises.add(['chatBoxesFetched', 'chatBoxesInitialized', 'privateChatsAutoJoined']);
+    _converse.api.promises.add(['chatBoxesFetched', 'ehatBoxesInitialized', 'privateChatsAutoJoined']);
 
     function openChat(jid) {
       if (!utils.isValidJID(jid)) {
@@ -62086,6 +62086,20 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
           return true;
         }, null, 'message', 'chat');
 
+        _converse.connection.addHandler(stanza => {
+          // Message receipts are usually without the `type` attribute. See #1353
+          if (!_.isNull(stanza.getAttribute('type'))) {
+            // TODO: currently Strophe has no way to register a handler
+            // for stanzas without a `type` attribute.
+            // We could update it to accept null to mean no attribute,
+            // but that would be a backward-incompatible chnge
+            return true; // Gets handled above.
+          }
+
+          this.onMessage(stanza);
+          return true;
+        }, Strophe.NS.RECEIPTS, 'message');
+
         _converse.connection.addHandler(stanza => {
           this.onErrorMessage(stanza);
           return true;
@@ -62202,7 +62216,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
           // XXX: Ideally we wouldn't have to check for headline
           // messages, but Prosody sends headline messages with the
           // wrong type ('chat'), so we need to filter them out here.
-          _converse.log(`onMessage: Ignoring incoming headline message sent with type 'chat' from JID: ${stanza.getAttribute('from')}`, Strophe.LogLevel.INFO);
+          _converse.log(`onMessage: Ignoring incoming headline message from JID: ${stanza.getAttribute('from')}`, Strophe.LogLevel.INFO);
 
           return true;
         }
@@ -69597,7 +69611,7 @@ u.isOnlyChatStateNotification = function (attrs) {
 };
 
 u.isHeadlineMessage = function (_converse, message) {
-  var from_jid = message.getAttribute('from');
+  const from_jid = message.getAttribute('from');
 
   if (message.getAttribute('type') === 'headline') {
     return true;
@@ -69605,7 +69619,7 @@ u.isHeadlineMessage = function (_converse, message) {
 
   const chatbox = _converse.chatboxes.get(strophe_js__WEBPACK_IMPORTED_MODULE_2__["Strophe"].getBareJidFromJid(from_jid));
 
-  if (chatbox && chatbox.get('type') === 'chatroom') {
+  if (chatbox && chatbox.get('type') === _converse.CHATROOMS_TYPE) {
     return false;
   }
 

spec/messages.js

@@ -7,14 +7,7 @@
         ], factory);
 } (this, function ($, jasmine, mock, test_utils) {
     "use strict";
-    const _ = converse.env._;
-    const sizzle = converse.env.sizzle;
-    const $iq = converse.env.$iq;
-    const $msg = converse.env.$msg;
-    const $pres = converse.env.$pres;
-    const Strophe = converse.env.Strophe;
-    const Promise = converse.env.Promise;
-    const moment = converse.env.moment;
+    const { Backbone, Promise, Strophe, $iq, $msg, $pres, b64_sha1, moment, sizzle, _ } = converse.env;
     const u = converse.env.utils;
 
 
@@ -490,7 +483,7 @@
                 }).c('body').t("This headline message will not be shown").tree();
             _converse.chatboxes.onMessage(msg);
             expect(_converse.log.calledWith(
-                "onMessage: Ignoring incoming headline message sent with type 'chat' from JID: localhost",
+                "onMessage: Ignoring incoming headline message from JID: localhost",
                 Strophe.LogLevel.INFO
             )).toBeTruthy();
             expect(u.isHeadlineMessage.called).toBeTruthy();
@@ -1304,16 +1297,39 @@
             const chatbox = _converse.chatboxes.get(contact_jid);
             expect(chatbox).toBeDefined();
             await new Promise((resolve, reject) => view.once('messageInserted', resolve));
-            const msg_obj = chatbox.messages.models[0];
-            const msg_id = msg_obj.get('msgid');
-            const msg = $msg({
+            let msg_obj = chatbox.messages.models[0];
+            let msg_id = msg_obj.get('msgid');
+            let msg = $msg({
                     'from': contact_jid,
                     'to': _converse.connection.jid,
                     'id': u.getUniqueId(),
                 }).c('received', {'id': msg_id, xmlns: Strophe.NS.RECEIPTS}).up().tree();
-            _converse.chatboxes.onMessage(msg);
+            _converse.connection._dataRecv(test_utils.createRequest(msg));
             await new Promise((resolve, reject) => view.model.messages.once('rendered', resolve));
             expect(view.el.querySelectorAll('.chat-msg__receipt').length).toBe(1);
+
+            // Also handle receipts with type 'chat'. See #1353
+            spyOn(_converse.chatboxes, 'onMessage').and.callThrough();
+            textarea.value = 'Another message';
+            view.keyPressed({
+                target: textarea,
+                preventDefault: _.noop,
+                keyCode: 13 // Enter
+            });
+            await new Promise((resolve, reject) => view.once('messageInserted', resolve));
+
+            msg_obj = chatbox.messages.models[1];
+            msg_id = msg_obj.get('msgid');
+            msg = $msg({
+                    'from': contact_jid,
+                    'type': 'chat',
+                    'to': _converse.connection.jid,
+                    'id': u.getUniqueId(),
+                }).c('received', {'id': msg_id, xmlns: Strophe.NS.RECEIPTS}).up().tree();
+            _converse.connection._dataRecv(test_utils.createRequest(msg));
+            await new Promise((resolve, reject) => view.model.messages.once('rendered', resolve));
+            expect(view.el.querySelectorAll('.chat-msg__receipt').length).toBe(2);
+            expect(_converse.chatboxes.onMessage.calls.count()).toBe(1);
             done();
         }));
 

src/headless/converse-chatboxes.js

@@ -40,7 +40,7 @@ converse.plugins.add('converse-chatboxes', {
         });
         _converse.api.promises.add([
             'chatBoxesFetched',
-            'chatBoxesInitialized',
+            'ehatBoxesInitialized',
             'privateChatsAutoJoined'
         ]);
 
@@ -635,6 +635,20 @@ converse.plugins.add('converse-chatboxes', {
                     this.onMessage(stanza);
                     return true;
                 }, null, 'message', 'chat');
+
+                _converse.connection.addHandler(stanza => {
+                    // Message receipts are usually without the `type` attribute. See #1353
+                    if (!_.isNull(stanza.getAttribute('type'))) {
+                        // TODO: currently Strophe has no way to register a handler
+                        // for stanzas without a `type` attribute.
+                        // We could update it to accept null to mean no attribute,
+                        // but that would be a backward-incompatible chnge
+                        return true; // Gets handled above.
+                    }
+                    this.onMessage(stanza);
+                    return true;
+                }, Strophe.NS.RECEIPTS, 'message');
+
                 _converse.connection.addHandler(stanza => {
                     this.onErrorMessage(stanza);
                     return true;
@@ -740,7 +754,7 @@ converse.plugins.add('converse-chatboxes', {
                     // messages, but Prosody sends headline messages with the
                     // wrong type ('chat'), so we need to filter them out here.
                     _converse.log(
-                        `onMessage: Ignoring incoming headline message sent with type 'chat' from JID: ${stanza.getAttribute('from')}`,
+                        `onMessage: Ignoring incoming headline message from JID: ${stanza.getAttribute('from')}`,
                         Strophe.LogLevel.INFO
                     );
                     return true;

src/headless/utils/core.js

@@ -88,12 +88,12 @@ u.isOnlyChatStateNotification = function (attrs) {
 };
 
 u.isHeadlineMessage = function (_converse, message) {
-    var from_jid = message.getAttribute('from');
+    const from_jid = message.getAttribute('from');
     if (message.getAttribute('type') === 'headline') {
         return true;
     }
     const chatbox = _converse.chatboxes.get(Strophe.getBareJidFromJid(from_jid));
-    if (chatbox && chatbox.get('type') === 'chatroom') {
+    if (chatbox && chatbox.get('type') === _converse.CHATROOMS_TYPE) {
         return false;
     }
     if (message.getAttribute('type') !== 'error' &&