From 5d8e5468baf4abb1191c24533a7f7db246645cc0 Mon Sep 17 00:00:00 2001 From: JC Brand Date: Thu, 10 Sep 2020 10:09:30 +0200 Subject: [PATCH] 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 | 2 +- src/converse-notification.js | 100 ++++++++++++++++++---------------- src/headless/converse-chat.js | 14 ++++- src/headless/converse-muc.js | 11 +++- 4 files changed, 76 insertions(+), 51 deletions(-) diff --git a/spec/notification.js b/spec/notification.js index fc6c1ac41..5c4c2a0ee 100644 --- a/spec/notification.js +++ b/spec/notification.js @@ -1,4 +1,4 @@ -/*global mock */ +/*global mock, converse */ const _ = converse.env._; const $msg = converse.env.$msg; diff --git a/src/converse-notification.js b/src/converse-notification.js index 7b60e511b..4fc81adf5 100644 --- a/src/converse-notification.js +++ b/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)) { - // We want to show notifications for headline messages. - return _converse.isMessageToHiddenChat(message); } - const is_me = Strophe.getBareJidFromJid(message.getAttribute('from')) === _converse.bare_jid; - return !u.isOnlyChatStateNotification(message) && - !u.isOnlyMessageDeliveryReceipt(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(attrs); + } + 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) { diff --git a/src/headless/converse-chat.js b/src/headless/converse-chat.js index 9ab24b193..4a520c124 100644 --- a/src/headless/converse-chat.js +++ b/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); } diff --git a/src/headless/converse-muc.js b/src/headless/converse-muc.js index 8a420cd49..dd7d5c72c 100644 --- a/src/headless/converse-muc.js +++ b/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); },