Refactor converse-notifications to used parsed message attributes

instead of querying the stanza.

Also fixes a bug where typing notifications triggered an AttributeError
inside `shouldNotifyOfGroupMessage`

spec/notification.js

@@ -1,4 +1,4 @@
-/*global mock */
+/*global mock, converse */
 
 const _ = converse.env._;
 const $msg = converse.env.$msg;

src/converse-notification.js

@@ -4,11 +4,10 @@
  * @license Mozilla Public License (MPLv2)
  */
 import log from "@converse/headless/log";
-import st from "@converse/headless/utils/stanza";
 import { __ } from './i18n';
 import { _converse, api, converse } from "@converse/headless/converse-core";
 
-const { Strophe, sizzle } = converse.env;
+const { Strophe } = converse.env;
 const u = converse.env.utils;
 
 const supports_html5_notification = "Notification" in window;
@@ -40,10 +39,14 @@ converse.plugins.add('converse-notification', {
          * Is this a group message for which we should notify the user?
          * @private
          * @method _converse#shouldNotifyOfGroupMessage
+         * @param { MUCMessageAttributes } attrs
          */
-        _converse.shouldNotifyOfGroupMessage = function (message, data) {
-            const jid = message.getAttribute('from');
-            const room_jid = Strophe.getBareJidFromJid(jid);
+        _converse.shouldNotifyOfGroupMessage = function (attrs) {
+            if (!attrs?.body) {
+                return false;
+            }
+            const jid = attrs.from;
+            const room_jid = attrs.from_muc;
             const notify_all = api.settings.get('notify_all_room_messages');
             const room = _converse.chatboxes.get(room_jid);
             const resource = Strophe.getResourceFromJid(jid);
@@ -52,11 +55,10 @@ converse.plugins.add('converse-notification', {
             const nick = room.get('nick');
 
             if (api.settings.get('notify_nicknames_without_references')) {
-                const body = message.querySelector('body');
-                is_mentioned = (new RegExp(`\\b${nick}\\b`)).test(body.textContent);
+                is_mentioned = (new RegExp(`\\b${nick}\\b`)).test(attrs.body);
             }
 
-            const is_referenced = data.attrs.references.map(r => r.value).includes(nick);
+            const is_referenced = attrs.references.map(r => r.value).includes(nick);
             const is_not_mine = sender !== nick;
             const should_notify_user = notify_all === true
                 || (Array.isArray(notify_all) && notify_all.includes(room_jid))
@@ -65,33 +67,38 @@ converse.plugins.add('converse-notification', {
             return is_not_mine && !!should_notify_user;
         };
 
-        _converse.isMessageToHiddenChat = function (message) {
-            if (_converse.isUniView()) {
-                const jid = Strophe.getBareJidFromJid(message.getAttribute('from'));
-                const view = _converse.chatboxviews.get(jid);
-                if (view) {
-                    return view.model.get('hidden') || _converse.windowState === 'hidden' || !u.isVisible(view.el);
-                }
-                return true;
-            }
-            return _converse.windowState === 'hidden';
-        }
+        /**
+         * Given parsed attributes for a message stanza, get the related
+         * chatbox and check whether it's hidden.
+         * @private
+         * @method _converse#isMessageToHiddenChat
+         * @param { MUCMessageAttributes } attrs
+         */
+        _converse.isMessageToHiddenChat = function (attrs) {
+            return _converse.chatboxes.get(attrs.from)?.isHidden() ?? false;
+        };
 
-        _converse.shouldNotifyOfMessage = function (message, data) {
-            const forwarded = message.querySelector('forwarded');
-            if (forwarded !== null) {
+        /**
+         * @private
+         * @method _converse#shouldNotifyOfMessage
+         * @param { MessageData|MUCMessageData } data
+         */
+        _converse.shouldNotifyOfMessage = function (data) {
+            const { attrs, stanza } = data;
+            if (!attrs || stanza.querySelector('forwarded') !== null) {
                 return false;
-            } else if (message.getAttribute('type') === 'groupchat') {
-                return _converse.shouldNotifyOfGroupMessage(message, data);
-            } else if (st.isHeadline(message)) {
+            }
+            if (attrs['type'] === 'groupchat') {
+                return _converse.shouldNotifyOfGroupMessage(attrs);
+            } else if (attrs.is_headline) {
                 // We want to show notifications for headline messages.
-                return _converse.isMessageToHiddenChat(message);
+                return _converse.isMessageToHiddenChat(attrs);
             }
-            const is_me = Strophe.getBareJidFromJid(message.getAttribute('from')) === _converse.bare_jid;
-            return !u.isOnlyChatStateNotification(message) &&
-                !u.isOnlyMessageDeliveryReceipt(message) &&
+            const is_me = Strophe.getBareJidFromJid(attrs.from) === _converse.bare_jid;
+            return !u.isOnlyChatStateNotification(stanza) &&
+                !u.isOnlyMessageDeliveryReceipt(stanza) &&
                 !is_me &&
-                (api.settings.get('show_desktop_notifications') === 'all' || _converse.isMessageToHiddenChat(message));
+                (api.settings.get('show_desktop_notifications') === 'all' || _converse.isMessageToHiddenChat(attrs));
         };
 
 
@@ -129,16 +136,21 @@ converse.plugins.add('converse-notification', {
          * Shows an HTML5 Notification with the passed in message
          * @private
          * @method _converse#showMessageNotification
-         * @param { String } message
+         * @param { MessageData|MUCMessageData } data
          */
-        _converse.showMessageNotification = function (message) {
+        _converse.showMessageNotification = function (data) {
+            const { attrs } = data;
+            if (attrs.is_error) {
+                return;
+            }
+
             if (!_converse.areDesktopNotificationsEnabled()) {
                 return;
             }
             let title, roster_item;
-            const full_from_jid = message.getAttribute('from'),
+            const full_from_jid = attrs.from,
                   from_jid = Strophe.getBareJidFromJid(full_from_jid);
-            if (message.getAttribute('type') === 'headline') {
+            if (attrs.type === 'headline') {
                 if (!from_jid.includes('@') || api.settings.get("allow_non_roster_messaging")) {
                     title = __("Notification from %1$s", from_jid);
                 } else {
@@ -147,7 +159,7 @@ converse.plugins.add('converse-notification', {
             } else if (!from_jid.includes('@')) {
                 // workaround for Prosody which doesn't give type "headline"
                 title = __("Notification from %1$s", from_jid);
-            } else if (message.getAttribute('type') === 'groupchat') {
+            } else if (attrs.type === 'groupchat') {
                 title = __("%1$s says", Strophe.getResourceFromJid(full_from_jid));
             } else {
                 if (_converse.roster === undefined) {
@@ -165,11 +177,8 @@ converse.plugins.add('converse-notification', {
                     }
                 }
             }
-            // TODO: we should suppress notifications if we cannot decrypt
-            // the message...
-            const body = sizzle(`encrypted[xmlns="${Strophe.NS.OMEMO}"]`, message).length ?
-                         __('OMEMO Message received') :
-                         message.querySelector('body')?.textContent;
+
+            const body = attrs.is_encrypted ? __('Encrypted message received') : attrs.body;
             if (!body) {
                 return;
             }
@@ -255,20 +264,19 @@ converse.plugins.add('converse-notification', {
             /* Event handler for the on('message') event. Will call methods
              * to play sounds and show HTML5 notifications.
              */
-            const message = data.stanza;
-            if (!_converse.shouldNotifyOfMessage(message, data)) {
+            if (!_converse.shouldNotifyOfMessage(data)) {
                 return false;
             }
             /**
              * Triggered when a notification (sound or HTML5 notification) for a new
              * message has will be made.
              * @event _converse#messageNotification
-             * @type { XMLElement }
+             * @type { MessageData|MUCMessageData}
              * @example _converse.api.listen.on('messageNotification', stanza => { ... });
              */
-            api.trigger('messageNotification', message);
+            api.trigger('messageNotification', data);
             _converse.playSoundNotification();
-            _converse.showMessageNotification(message);
+            _converse.showMessageNotification(data);
         };
 
         _converse.handleContactRequestNotification = function (contact) {

src/headless/converse-chat.js

@@ -1141,7 +1141,7 @@ converse.plugins.add('converse-chat', {
              */
             isHidden () {
                 // Note: This methods gets overridden by converse-minimize
-                const hidden = ['mobile', 'fullscreen'].includes(api.settings.get("view_mode")) && this.get('hidden');
+                const hidden = _converse.isUniView() && this.get('hidden');
                 return hidden || this.isScrolledUp() || _converse.windowState === 'hidden';
             },
 
@@ -1214,14 +1214,22 @@ converse.plugins.add('converse-chat', {
             const has_body = !!sizzle(`body, encrypted[xmlns="${Strophe.NS.OMEMO}"]`, stanza).length;
             const chatbox = await api.chats.get(attrs.contact_jid, {'nickname': attrs.nick }, has_body);
             await chatbox?.queueMessage(attrs);
+            /**
+             * An object containing the original message stanza, as well as the
+             * parsed attributes.
+             * @typedef { Object } MessageData
+             * @property { XMLElement } stanza
+             * @property { MessageAttributes } stanza
+             */
+            const data = {stanza, attrs};
             /**
              * Triggered when a message stanza is been received and processed.
              * @event _converse#message
              * @type { object }
-             * @property { XMLElement } stanza
+             * @property { MessageData|MUCMessageData } data
              * @example _converse.api.listen.on('message', obj => { ... });
              */
-            api.trigger('message', {stanza, attrs});
+            api.trigger('message', data);
         }
 
 

src/headless/converse-muc.js

@@ -671,8 +671,17 @@ converse.plugins.add('converse-muc', {
                 }
                 this.createInfoMessages(stanza);
                 this.fetchFeaturesIfConfigurationChanged(stanza);
+
+                /**
+                 * An object containing the original groupchat message stanza,
+                 * as well as the parsed attributes.
+                 * @typedef { Object } MUCMessageData
+                 * @property { XMLElement } stanza
+                 * @property { MUCMessageAttributes } stanza
+                 */
                 const attrs = await st.parseMUCMessage(stanza, this, _converse);
-                api.trigger('message', {stanza, attrs});
+                const data = {stanza, attrs};
+                api.trigger('message', data);
                 return attrs && this.queueMessage(attrs);
             },