diff --git a/CHANGES.md b/CHANGES.md index 1d5c06a9c..e0218e303 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,7 @@ - New API method [\_converse.api.disco.features.get](https://conversejs.org/docs/html/api/-_converse.api.disco.features.html#.get) - New config setting [muc_show_join_leave_status](https://conversejs.org/docs/html/configuration.html#muc-show-join-leave-status) - New event: `chatBoxBlurred`. +- Properly handle message correction being received before the corrected message - #1296: `embedded` view mode shows `chatbox-navback` arrow in header - #1465: When highlighting a roster contact, they're incorrectly shown as online - #1532: Converse reloads on enter pressed in the filter box diff --git a/sass/_messages.scss b/sass/_messages.scss index cce540c65..c077b2cc5 100644 --- a/sass/_messages.scss +++ b/sass/_messages.scss @@ -1,4 +1,9 @@ #conversejs { + .older-msg { + time { + font-weight: bold; + } + } .message { .mention { font-weight: bold; diff --git a/spec/messages.js b/spec/messages.js index 2ea5da989..48798838e 100644 --- a/spec/messages.js +++ b/spec/messages.js @@ -76,8 +76,11 @@ const corrected_message = view.model.messages.at(0); expect(corrected_message.get('msgid')).toBe(first_msg.get('msgid')); expect(corrected_message.get('correcting')).toBe(false); - expect(corrected_message.get('older_versions').length).toBe(1); - expect(corrected_message.get('older_versions')[0]).toBe('But soft, what light through yonder airlock breaks?'); + + const older_versions = corrected_message.get('older_versions'); + const keys = Object.keys(older_versions); + expect(keys.length).toBe(1); + expect(older_versions[keys[0]]).toBe('But soft, what light through yonder airlock breaks?'); expect(view.el.querySelectorAll('.chat-msg').length).toBe(1); expect(u.hasClass('correcting', view.el.querySelector('.chat-msg'))).toBe(false); @@ -181,8 +184,11 @@ const corrected_message = view.model.messages.at(0); expect(corrected_message.get('msgid')).toBe(first_msg.get('msgid')); expect(corrected_message.get('correcting')).toBe(false); - expect(corrected_message.get('older_versions').length).toBe(1); - expect(corrected_message.get('older_versions')[0]).toBe('But soft, what light through yonder airlock breaks?'); + + const older_versions = corrected_message.get('older_versions'); + const keys = Object.keys(older_versions); + expect(keys.length).toBe(1); + expect(older_versions[keys[0]]).toBe('But soft, what light through yonder airlock breaks?'); expect(view.el.querySelectorAll('.chat-msg').length).toBe(1); await test_utils.waitUntil(() => (u.hasClass('correcting', view.el.querySelector('.chat-msg')) === false), 500); @@ -1472,8 +1478,8 @@ await test_utils.waitUntil(() => u.isVisible(modal.el), 1000); const older_msgs = modal.el.querySelectorAll('.older-msg'); expect(older_msgs.length).toBe(2); - expect(older_msgs[0].textContent).toBe('But soft, what light through yonder airlock breaks?'); - expect(older_msgs[1].textContent).toBe('But soft, what light through yonder chimney breaks?'); + expect(older_msgs[0].childNodes[0].nodeName).toBe('TIME'); + expect(older_msgs[0].childNodes[1].textContent).toBe(': But soft, what light through yonder airlock breaks?'); done(); })); @@ -2425,8 +2431,10 @@ await test_utils.waitUntil(() => u.isVisible(modal.el), 1000); const older_msgs = modal.el.querySelectorAll('.older-msg'); expect(older_msgs.length).toBe(2); - expect(older_msgs[0].textContent).toBe('But soft, what light through yonder airlock breaks?'); - expect(older_msgs[1].textContent).toBe('But soft, what light through yonder chimney breaks?'); + expect(older_msgs[0].childNodes[1].textContent).toBe(': But soft, what light through yonder airlock breaks?'); + expect(older_msgs[0].childNodes[0].nodeName).toBe('TIME'); + expect(older_msgs[1].childNodes[0].nodeName).toBe('TIME'); + expect(older_msgs[1].childNodes[1].textContent).toBe(': But soft, what light through yonder chimney breaks?'); done(); })); @@ -2495,8 +2503,11 @@ const corrected_message = view.model.messages.at(0); expect(corrected_message.get('msgid')).toBe(first_msg.get('msgid')); expect(corrected_message.get('correcting')).toBe(false); - expect(corrected_message.get('older_versions').length).toBe(1); - expect(corrected_message.get('older_versions')[0]).toBe('But soft, what light through yonder airlock breaks?'); + + const older_versions = corrected_message.get('older_versions'); + const keys = Object.keys(older_versions); + expect(keys.length).toBe(1); + expect(older_versions[keys[0]]).toBe('But soft, what light through yonder airlock breaks?'); expect(view.el.querySelectorAll('.chat-msg').length).toBe(1); expect(u.hasClass('correcting', view.el.querySelector('.chat-msg'))).toBe(false); diff --git a/src/converse-message-view.js b/src/converse-message-view.js index 3e52e5181..8de88da1e 100644 --- a/src/converse-message-view.js +++ b/src/converse-message-view.js @@ -69,7 +69,8 @@ converse.plugins.add('converse-message-view', { toHTML () { return tpl_message_versions_modal(Object.assign( this.model.toJSON(), { - '__': __ + '__': __, + 'dayjs': dayjs })); } }); diff --git a/src/headless/converse-chatboxes.js b/src/headless/converse-chatboxes.js index e32b572eb..31a26fc16 100644 --- a/src/headless/converse-chatboxes.js +++ b/src/headless/converse-chatboxes.js @@ -382,30 +382,52 @@ converse.plugins.add('converse-chatboxes', { } }, - handleMessageCorrection (stanza) { - const replace = sizzle(`replace[xmlns="${Strophe.NS.MESSAGE_CORRECT}"]`, stanza).pop(); - if (replace) { - const msgid = replace && replace.getAttribute('id') || stanza.getAttribute('id'), - message = msgid && this.messages.findWhere({msgid}); - - if (!message) { - // XXX: Looks like we received a correction for a - // non-existing message, probably due to MAM. - // Not clear what can be done about this... we'll - // just create it as a separate message for now. - return false; - } - const older_versions = message.get('older_versions') || []; - older_versions.push(message.get('message')); - message.save({ - 'message': this.getMessageBody(stanza), - 'references': this.getReferencesFromStanza(stanza), - 'older_versions': older_versions, - 'edited': (new Date()).toISOString() - }); - return true; + /** + * If the passed in `message` stanza contains an + * [XEP-0308](https://xmpp.org/extensions/xep-0308.html#usecase) + * `` element, return its `id` attribute. + * @private + * @method _converse.ChatBox#getReplaceId + * @param { XMLElement } stanza + */ + getReplaceId (stanza) { + const el = sizzle(`replace[xmlns="${Strophe.NS.MESSAGE_CORRECT}"]`, stanza).pop(); + if (el) { + return el.getAttribute('id'); } - return false; + }, + + /** + * Determine 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#correctMessage + * @param { object } attrs - Attributes representing a received + * message, as returned by + * {@link _converse.ChatBox.getMessageAttributesFromStanza} + */ + correctMessage (attrs) { + if (!attrs.msgid || !attrs.from) { + return; + } + const message = this.messages.findWhere({'msgid': attrs.msgid, '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 already + older_versions[attrs.time] = attrs['message']; + message.save({'older_versions': older_versions}); + } else { + // This is a correction of an earlier message we already received + older_versions[message.get('time')] = message.get('message'); + attrs = Object.assign(attrs, {'older_versions': older_versions}); + message.save(attrs); + } + return message; }, async getDuplicateMessage (stanza) { @@ -619,8 +641,8 @@ converse.plugins.add('converse-chatboxes', { const attrs = this.getOutgoingMessageAttributes(text, spoiler_hint); let message = this.messages.findWhere('correcting') if (message) { - const older_versions = message.get('older_versions') || []; - older_versions.push(message.get('message')); + const older_versions = message.get('older_versions') || {}; + older_versions[message.get('time')] = message.get('message'); message.save({ 'correcting': false, 'edited': (new Date()).toISOString(), @@ -772,15 +794,17 @@ converse.plugins.add('converse-chatboxes', { * message stanza, if it was contained, otherwise it's the message stanza itself. */ getMessageAttributesFromStanza (stanza, original_stanza) { - const spoiler = sizzle(`spoiler[xmlns="${Strophe.NS.SPOILER}"]`, original_stanza).pop(), - delay = sizzle(`delay[xmlns="${Strophe.NS.DELAY}"]`, original_stanza).pop(), - text = this.getMessageBody(stanza) || undefined, - chat_state = stanza.getElementsByTagName(_converse.COMPOSING).length && _converse.COMPOSING || + const spoiler = sizzle(`spoiler[xmlns="${Strophe.NS.SPOILER}"]`, original_stanza).pop(); + const delay = sizzle(`delay[xmlns="${Strophe.NS.DELAY}"]`, original_stanza).pop(); + const text = this.getMessageBody(stanza) || undefined; + const chat_state = stanza.getElementsByTagName(_converse.COMPOSING).length && _converse.COMPOSING || stanza.getElementsByTagName(_converse.PAUSED).length && _converse.PAUSED || stanza.getElementsByTagName(_converse.INACTIVE).length && _converse.INACTIVE || stanza.getElementsByTagName(_converse.ACTIVE).length && _converse.ACTIVE || stanza.getElementsByTagName(_converse.GONE).length && _converse.GONE; + const replaced_id = this.getReplaceId(stanza) + const msgid = replaced_id || stanza.getAttribute('id') || original_stanza.getAttribute('id'); const attrs = Object.assign({ 'chat_state': chat_state, 'is_archived': this.isArchived(original_stanza), @@ -788,7 +812,7 @@ converse.plugins.add('converse-chatboxes', { 'is_spoiler': !_.isNil(spoiler), 'is_single_emoji': text ? u.isSingleEmoji(text) : false, 'message': text, - 'msgid': stanza.getAttribute('id'), + 'msgid': msgid, 'references': this.getReferencesFromStanza(stanza), 'subject': _.propertyOf(stanza.querySelector('subject'))('textContent'), 'thread': _.propertyOf(stanza.querySelector('thread'))('textContent'), @@ -796,7 +820,6 @@ converse.plugins.add('converse-chatboxes', { 'type': stanza.getAttribute('type') }, this.getStanzaIDs(original_stanza)); - if (attrs.type === 'groupchat') { attrs.from = stanza.getAttribute('from'); attrs.nick = Strophe.unescapeNode(Strophe.getResourceFromJid(attrs.from)); @@ -818,8 +841,13 @@ converse.plugins.add('converse-chatboxes', { if (spoiler) { attrs.spoiler_hint = spoiler.textContent.length > 0 ? spoiler.textContent : ''; } + if (replaced_id) { + attrs['edited'] = (new Date()).toISOString(); + } // We prefer to use one of the XEP-0359 unique and stable stanza IDs as the Model id, to avoid duplicates. - attrs['id'] = attrs['origin_id'] || attrs[`stanza_id ${attrs.from}`] || _converse.connection.getUniqueId(); + attrs['id'] = attrs['origin_id'] || + attrs[`stanza_id ${attrs.from}`] || + _converse.connection.getUniqueId(); return attrs; }, @@ -1021,13 +1049,12 @@ converse.plugins.add('converse-chatboxes', { chatbox.updateMessage(message, original_stanza); } if (!message && - !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt (stanza, from_jid, is_carbon, is_me, is_mam) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact, is_mam)) { const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza); if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) { - const msg = chatbox.messages.create(attrs); + const msg = chatbox.correctMessage(attrs) || chatbox.messages.create(attrs); chatbox.incrementUnreadMsgCounter(msg); } } diff --git a/src/headless/converse-core.js b/src/headless/converse-core.js index ec13e9ba5..8f40d05d1 100644 --- a/src/headless/converse-core.js +++ b/src/headless/converse-core.js @@ -9,7 +9,7 @@ import Backbone from "backbone"; import BrowserStorage from "backbone.browserStorage"; import Promise from "es6-promise/dist/es6-promise.auto"; import _ from "./lodash.noconflict"; -import advancedFormat from 'dayjs/plugin/advancedFormat' +import advancedFormat from 'dayjs/plugin/advancedFormat'; import dayjs from "dayjs"; import i18n from "./i18n"; import pluggable from "pluggable.js/src/pluggable"; @@ -19,7 +19,7 @@ import u from "@converse/headless/utils/core"; Backbone = Backbone.noConflict(); -dayjs.extend(advancedFormat) +dayjs.extend(advancedFormat); // Strophe globals const b64_sha1 = SHA1.b64_sha1; diff --git a/src/headless/converse-muc.js b/src/headless/converse-muc.js index c2482276a..f9aad5d8a 100644 --- a/src/headless/converse-muc.js +++ b/src/headless/converse-muc.js @@ -1158,19 +1158,19 @@ converse.plugins.add('converse-muc', { this.updateMessage(message, original_stanza); } if (message || - this.handleMessageCorrection(stanza) || this.isReceipt(stanza) || this.isChatMarker(stanza)) { return _converse.api.trigger('message', {'stanza': original_stanza}); } - const attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza); + let attrs = await this.getMessageAttributesFromStanza(stanza, original_stanza); if (attrs.nick && !this.subjectChangeHandled(attrs) && !this.ignorableCSN(attrs) && (attrs['chat_state'] || !u.isEmptyMessage(attrs))) { - const msg = this.messages.create(this.addOccupantData(attrs)); + attrs = this.addOccupantData(attrs); + const msg = this.correctMessage(attrs) || this.messages.create(attrs); this.incrementUnreadMsgCounter(msg); if (forwarded && msg && msg.get('sender') === 'me') { msg.save({'received': (new Date()).toISOString()}); diff --git a/src/templates/message_versions_modal.html b/src/templates/message_versions_modal.html index 1d908b88f..5dc286da8 100644 --- a/src/templates/message_versions_modal.html +++ b/src/templates/message_versions_modal.html @@ -7,7 +7,7 @@