Jelajahi Sumber

autocomplete: Use regex instead of hardcoded list...

to determine valid characters to form a boundary before an `@` mention

Also fixed an issue with mentions looking like they're part of URLs, by
first processing mentions separately.
JC Brand 4 tahun lalu
induk
melakukan
f86efca9a6

+ 1 - 1
CHANGES.md

@@ -3,7 +3,7 @@
 ## 8.0.0 (Unreleased)
 
 - #1083: Add support for XEP-0393 Message Styling
-- #2275: Allow selected characters to precede a mention
+- #2275: Allow punctuation to immediately precede a mention
 - Bugfix: `null` inserted by emoji picker and can't switch between skintones
 - New configuration setting: [show_tab_notifications](https://conversejs.org/docs/html/configuration.html#show-tab-notifications)
 

+ 19 - 6
spec/mentions.js

@@ -52,7 +52,7 @@ describe("An incoming groupchat message", function () {
                 }))
             );
         });
-        const msg = $msg({
+        let msg = $msg({
                 from: 'lounge@montague.lit/gibson',
                 id: u.getUniqueId(),
                 to: 'romeo@montague.lit',
@@ -62,7 +62,22 @@ describe("An incoming groupchat message", function () {
                 .c('reference', {'xmlns':'urn:xmpp:reference:0', 'begin':'11', 'end':'14', 'type':'mention', 'uri':'xmpp:romeo@montague.lit'}).up()
                 .c('reference', {'xmlns':'urn:xmpp:reference:0', 'begin':'15', 'end':'23', 'type':'mention', 'uri':'xmpp:mr.robot@montague.lit'}).nodeTree;
         await view.model.handleMessageStanza(msg);
-        const message = await u.waitUntil(() => view.el.querySelector('.chat-msg__text'));
+        let message = await u.waitUntil(() => view.el.querySelector('.chat-msg__text'));
+        expect(message.classList.length).toEqual(1);
+        expect(message.innerHTML.replace(/<!---->/g, '')).toBe(
+            'hello <span class="mention">z3r0</span> '+
+            '<span class="mention mention--self badge badge-info">tom</span> '+
+            '<span class="mention">mr.robot</span>, how are you?');
+
+        msg = $msg({
+                from: 'lounge@montague.lit/sw0rdf1sh',
+                id: u.getUniqueId(),
+                to: 'romeo@montague.lit',
+                type: 'groupchat'
+            }).c('body').t('https://conversejs.org/@gibson').up()
+                .c('reference', {'xmlns':'urn:xmpp:reference:0', 'begin':'23', 'end':'29', 'type':'mention', 'uri':'xmpp:gibson@montague.lit'}).nodeTree;
+        await view.model.handleMessageStanza(msg);
+        message = await u.waitUntil(() => view.el.querySelector('.chat-msg__text'));
         expect(message.classList.length).toEqual(1);
         expect(message.innerHTML.replace(/<!---->/g, '')).toBe(
             'hello <span class="mention">z3r0</span> '+
@@ -139,8 +154,7 @@ describe("A sent groupchat message", function () {
                     })));
             });
 
-            // Also check that nicks from received messages, (but for which
-            // we don't have occupant objects) can be mentioned.
+            // Also check that nicks from received messages, (but for which we don't have occupant objects) can be mentioned.
             const stanza = u.toStanza(`
                 <message xmlns="jabber:client"
                         from="${muc_jid}/gh0st"
@@ -200,8 +214,7 @@ describe("A sent groupchat message", function () {
             [text, references] = view.model.parseTextForReferences('https://example.org/@gibson')
             expect(text).toBe('https://example.org/@gibson');
             expect(references.length).toBe(0);
-            expect(references)
-                .toEqual([]);
+            expect(references).toEqual([]);
 
             [text, references] = view.model.parseTextForReferences('mail@gibson.com')
             expect(text).toBe('mail@gibson.com');

+ 6 - 10
src/converse-autocomplete.js

@@ -9,7 +9,6 @@
 import { Events } from '@converse/skeletor/src/events.js';
 import { converse } from "@converse/headless/converse-core";
 
-converse.MENTION_BOUNDARIES = ['"', '(', '<', '#', '!', '\\', '/', '+', '~', '[', '{', '^', '>'];
 const u = converse.env.utils;
 
 
@@ -96,9 +95,8 @@ const helpers = {
         return s.replace(/[-\\^$*+?.()|[\]{}]/g, "\\$&");
     },
 
-    isMention (word, ac_triggers, mention_boundaries) {
-        return (ac_triggers.includes(word[0]) ||
-        (mention_boundaries.includes(word[0]) && ac_triggers.includes(word[1])));
+    isMention (word, ac_triggers) {
+        return (ac_triggers.includes(word[0]) || u.isMentionBoundary(word[0]) && ac_triggers.includes(word[1]));
     }
 }
 
@@ -251,7 +249,7 @@ export class AutoComplete {
 
     insertValue (suggestion) {
         if (this.match_current_word) {
-            u.replaceCurrentWord(this.input, suggestion.value, converse.MENTION_BOUNDARIES);
+            u.replaceCurrentWord(this.input, suggestion.value);
         } else {
             this.input.value = suggestion.value;
         }
@@ -371,7 +369,7 @@ export class AutoComplete {
             this.auto_completing = true;
         } else if (ev.key === "Backspace") {
             const word = u.getCurrentWord(ev.target, ev.target.selectionEnd-1);
-            if (helpers.isMention(word, this.ac_triggers, converse.MENTION_BOUNDARIES)) {
+            if (helpers.isMention(word, this.ac_triggers)) {
                 this.auto_completing = true;
             }
         }
@@ -393,11 +391,11 @@ export class AutoComplete {
         }
 
         let value = this.match_current_word ? u.getCurrentWord(this.input) : this.input.value;
-        const contains_trigger = helpers.isMention(value, this.ac_triggers, converse.MENTION_BOUNDARIES);
+        const contains_trigger = helpers.isMention(value, this.ac_triggers);
         if (contains_trigger) {
             this.auto_completing = true;
             if (!this.include_triggers.includes(ev.key)) {
-                value = converse.MENTION_BOUNDARIES.includes(value[0])
+                value = u.isMentionBoundary(value[0])
                     ? value.slice('2')
                     : value.slice('1');
             }
@@ -445,5 +443,3 @@ converse.plugins.add("converse-autocomplete", {
         _converse.AutoComplete = AutoComplete;
     }
 });
-
-

+ 16 - 12
src/headless/converse-muc.js

@@ -20,6 +20,7 @@ import p from "./utils/parse-helpers";
 export const ROLES = ['moderator', 'participant', 'visitor'];
 export const AFFILIATIONS = ['owner', 'admin', 'member', 'outcast', 'none'];
 
+
 converse.MUC_TRAFFIC_STATES = ['entered', 'exited'];
 converse.MUC_ROLE_CHANGES = ['op', 'deop', 'voice', 'mute'];
 
@@ -963,9 +964,7 @@ converse.plugins.add('converse-muc', {
             getAllKnownNicknamesRegex () {
                 const longNickString = this.getAllKnownNicknames().join('|');
                 const escapedLongNickString = p.escapeRegexString(longNickString)
-                const mention_boundaries = converse.MENTION_BOUNDARIES.join('|');
-                const escaped_mention_boundaries = p.escapeRegexString(mention_boundaries);
-                return RegExp(`(?:\\s|^)[${escaped_mention_boundaries}]?@(${escapedLongNickString})(?![\\w@-])`, 'ig');
+                return RegExp(`(?:\\p{P}|\\p{Z}|^)@(${escapedLongNickString})(?![\\w@-])`, 'uig');
             },
 
             getOccupantByJID (jid) {
@@ -976,14 +975,18 @@ converse.plugins.add('converse-muc', {
                 return this.occupants.findOccupant({ nick });
             },
 
-            parseTextForReferences (original_message) {
-                if (!original_message) return ['', []];
-                const findRegexInMessage = p.matchRegexInText(original_message);
-                const raw_mentions = findRegexInMessage(p.mention_regex);
-                if (!raw_mentions) return [original_message, []];
+            /**
+             * Given a text message, look for `@` mentions and turn them into
+             * XEP-0372 references
+             * @param { String } text
+             */
+            parseTextForReferences (text) {
+                const mentions_regex = /(\p{P}|\p{Z}|^)([@][\w_-]+(?:\.\w+)*)/ugi;
+                if (!text || !mentions_regex.test(text)) {
+                    return [text, []];
+                }
 
-                const known_nicknames = this.getAllKnownNicknames();
-                const getMatchingNickname = p.findFirstMatchInArray(known_nicknames);
+                const getMatchingNickname = p.findFirstMatchInArray(this.getAllKnownNicknames());
 
                 const uriFromNickname = nickname => {
                     const jid = this.get('jid');
@@ -1002,11 +1005,12 @@ converse.plugins.add('converse-muc', {
                     return { begin, end, value, type, uri }
                 }
 
-                const mentions = [...findRegexInMessage(this.getAllKnownNicknamesRegex())];
+                const regex = this.getAllKnownNicknamesRegex();
+                const mentions = [...text.matchAll(regex)].filter(m => !m[0].startsWith('/'));
                 const references = mentions.map(matchToReference);
 
                 const [updated_message, updated_references] = p.reduceTextFromReferences(
-                    original_message,
+                    text,
                     references
                 );
                 return [updated_message, updated_references];

+ 7 - 7
src/headless/utils/core.js

@@ -425,13 +425,13 @@ u.getCurrentWord = function (input, index, delineator) {
     return word;
 };
 
-u.replaceCurrentWord = function (input, new_value, mention_boundaries=[]) {
-    const caret = input.selectionEnd || undefined,
-        current_word = last(input.value.slice(0, caret).split(/\s/)),
-        value = input.value,
-        mention_boundary = mention_boundaries.includes(current_word[0])
-            ? current_word[0]
-            : '';
+u.isMentionBoundary = (s) => s !== '@' && RegExp(`(\\p{Z}|\\p{P})`, 'u').test(s);
+
+u.replaceCurrentWord = function (input, new_value) {
+    const caret = input.selectionEnd || undefined;
+    const current_word = last(input.value.slice(0, caret).split(/\s/));
+    const value = input.value;
+    const mention_boundary = u.isMentionBoundary(current_word[0]) ? current_word[0] : '';
     input.value = value.slice(0, caret - current_word.length) + mention_boundary + `${new_value} ` + value.slice(caret);
     const selection_end = caret - current_word.length + new_value.length + 1;
     input.selectionEnd = mention_boundary ? selection_end + 1 : selection_end;

+ 0 - 5
src/headless/utils/parse-helpers.js

@@ -6,11 +6,6 @@
  */
 const helpers = {};
 
-// Captures all mentions, but includes a space before the @
-helpers.mention_regex = /(?:\s|^)([@][\w_-]+(?:\.\w+)*)/gi;
-
-helpers.matchRegexInText = text => regex => text.matchAll(regex);
-
 const escapeRegexChars = (string, char) => string.replace(RegExp('\\' + char, 'ig'), '\\' + char);
 
 helpers.escapeCharacters = characters => string =>

+ 31 - 18
src/shared/message/text.js

@@ -112,8 +112,7 @@ export class MessageText extends String {
      * @param { Integer } offset - The index of the passed in text relative to
      *  the start of the message body text.
      */
-    async addEmojis (text, offset) {
-        await api.emojis.initialize();
+    addEmojis (text, offset) {
         const references = [...getShortnameReferences(text.toString()), ...getCodePointReferences(text.toString())];
         references.forEach(e => {
             this.addTemplateResult(
@@ -129,9 +128,10 @@ export class MessageText extends String {
      * rendering them.
      * @param { String } text
      * @param { Integer } offset - The index of the passed in text relative to
-     *  the start of the message body text.
+     *  the start of the MessageText.
      */
     addMentions (text, offset) {
+        offset += this.offset;
         if (!this.model.collection) {
             // This model doesn't belong to a collection anymore, so it must be
             // have been removed in the meantime and can be ignored.
@@ -193,6 +193,28 @@ export class MessageText extends String {
         }
     }
 
+
+    /**
+     * Look for plaintext (i.e. non-templated) sections of this MessageText
+     * instance and add references via the passed in function.
+     * @param { Function } func
+     */
+    addReferences (func) {
+        const payload = this.marshall();
+        let idx = 0; // The text index of the element in the payload
+        for (const text of payload) {
+            if (!text) {
+                continue
+            } else if (isString(text)) {
+                func.call(this, text, idx);
+                idx += text.length;
+            } else {
+                idx = text.end;
+            }
+        }
+    }
+
+
     /**
      * Parse the text and add template references for rendering the "rich" parts.
      *
@@ -214,21 +236,12 @@ export class MessageText extends String {
         await api.trigger('beforeMessageBodyTransformed', this, {'Synchronous': true});
 
         this.addStyling();
-        const payload = this.marshall();
-        let idx = 0; // The text index of the element in the payload
-        for (const text of payload) {
-            if (!text) {
-                continue
-            } else if (isString(text)) {
-                this.addHyperlinks(text, idx);
-                this.addMapURLs(text, idx);
-                await this.addEmojis(text, idx);
-                this.addMentions(text, this.offset+idx);
-                idx += text.length;
-            } else {
-                idx += text.end;
-            }
-        }
+        this.addReferences(this.addMentions);
+        this.addReferences(this.addHyperlinks);
+        this.addReferences(this.addMapURLs);
+
+        await api.emojis.initialize();
+        this.addReferences(this.addEmojis);
 
         /**
          * Synchronous event which provides a hook for transforming a chat message's body text