Skip to content

Commit

Permalink
Merge pull request #658 from matrix-org/hs/mentions-use-displaynames
Browse files Browse the repository at this point in the history 
Use localpart in plaintext so we get highlighted for pills
  • Loading branch information
@Half-Shot
Half-Shot authored Aug 23, 2018
2 parents eb84467 + 8686f87 commit 4789667
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 44 deletions.
2 changes: 1 addition & 1 deletion lib/bridge/IrcHandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ IrcHandler.prototype.onMessage = Promise.coroutine(function*(req, server, fromUs
}
}

mxAction.formatMentions(mapping);
yield mxAction.formatMentions(mapping, this.ircBridge.getAppServiceBridge().getIntent());

if (!mxAction) {
req.log.error("Couldn't map IRC action to matrix action");
Expand Down
4 changes: 3 additions & 1 deletion lib/bridge/MatrixHandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1653,7 +1653,9 @@ MatrixHandler.prototype.onUserQuery = function(req, userId) {
};

MatrixHandler.prototype.getMetrics = function(serverDomain) {
return this.metrics[serverDomain] || {};
const metrics = this.metrics[serverDomain] || {};
this.metrics[serverDomain] = {}
return metrics || {};
}

function reqHandler(req, promise) {
Expand Down
44 changes: 36 additions & 8 deletions lib/models/MatrixAction.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
/*eslint no-invalid-this: 0*/ // eslint doesn't understand Promise.coroutine wrapping
"use strict";
const ircFormatting = require("../irc/formatting");
const log = require("../logging").get("MatrixAction");
const ContentRepo = require("matrix-appservice-bridge").ContentRepo;
const escapeStringRegexp = require('escape-string-regexp');
const Promise = require("bluebird");

const ACTION_TYPES = ["message", "emote", "topic", "notice", "file", "image", "video", "audio"];
const EVENT_TO_TYPE = {
Expand Down Expand Up @@ -31,15 +33,15 @@ function MatrixAction(type, text, htmlText, timestamp) {
this.ts = timestamp || 0;
}

MatrixAction.prototype.formatMentions = function(nickUserIdMap) {
MatrixAction.prototype.formatMentions = Promise.coroutine(function*(nickUserIdMap, intent) {
const regexString = "(" +
Object.keys(nickUserIdMap).map((value) => escapeStringRegexp(value)).join("|")
+ ")";
const userRegex = new RegExp(regexString, "igm");
const usersRegex = MentionRegex(regexString);
const matched = new Set(); // lowercased nicknames we have matched already.
let match;
for (let i = 0; i < MAX_MATCHES && (match = userRegex.exec(this.text)) !== null; i++) {
let matchName = match[1];
for (let i = 0; i < MAX_MATCHES && (match = usersRegex.exec(this.text)) !== null; i++) {
let matchName = match[2];
// Deliberately have a minimum length to match on,
// so we don't match smaller nicks accidentally.
if (matchName.length < PILL_MIN_LENGTH_TO_MATCH || matched.has(matchName.toLowerCase())) {
Expand All @@ -64,14 +66,32 @@ MatrixAction.prototype.formatMentions = function(nickUserIdMap) {
this.htmlText = this.text;
}
userId = ircFormatting.escapeHtmlChars(userId);
this.htmlText = this.htmlText.replace(
new RegExp(escapeStringRegexp(matchName), "igm"),
`<a href="https://matrix.to/#/${userId}">${ircFormatting.escapeHtmlChars(matchName)}</a>`

/* Due to how Riot and friends do push notifications,
we need the plain text to match something.*/
let identifier;
try {
identifier = (yield intent.getProfileInfo(userId, 'displayname', true)).displayname;
}
catch (e) {
// This shouldn't happen, but let's not fail to match if so.
}

if (identifier === undefined) {
// Fallback to userid.
identifier = userId.substr(1, userId.indexOf(":")-1)
}

const regex = MentionRegex(escapeStringRegexp(matchName));
this.htmlText = this.htmlText.replace(regex,
`$1<a href="https://matrix.to/#/${userId}">`+
`${ircFormatting.escapeHtmlChars(identifier)}</a>`
);
this.text = this.text.replace(regex, `$1${identifier}`);
// Don't match this name twice, we've already replaced all entries.
matched.add(matchName.toLowerCase());
}
}
});

MatrixAction.fromEvent = function(client, event, mediaUrl) {
event.content = event.content || {};
Expand Down Expand Up @@ -128,4 +148,12 @@ MatrixAction.fromIrcAction = function(ircAction) {
}
};

function MentionRegex(matcher) {
const WORD_BOUNDARY = "^|\:|\#|```|\\s|$|,";
return new RegExp(
`(${WORD_BOUNDARY})(@?(${matcher}))(?=${WORD_BOUNDARY})`,
"igmu"
);
}

module.exports = MatrixAction;
148 changes: 114 additions & 34 deletions spec/unit/MatrixAction.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,29 +1,50 @@
"use strict";
const MatrixAction = require("../../lib/models/MatrixAction");

const FakeIntent = {
getProfileInfo: (userId) => {
return new Promise((resolve, reject) => {
if (userId === "@jc.denton:unatco.gov") {
resolve({displayname: "TheJCDenton"});
}
else if (userId === "@paul.denton:unatco.gov") {
resolve({displayname: "ThePaulDenton"});
}
else {
reject("This user was not found");
}
});
}
}

describe("MatrixAction", function() {

it("should not highlight mentions to text without mentions", () => {
let action = new MatrixAction("message", "Some text");
action.formatMentions({
return action.formatMentions({
"Some Person": "@foobar:localhost"
}, FakeIntent).then(() => {
expect(action.text).toEqual("Some text");
expect(action.htmlText).toBeUndefined();
});
expect(action.text).toEqual("Some text");
expect(action.htmlText).toBeUndefined();
});

it("should highlight a user", () => {
let action = new MatrixAction(
"message",
"JCDenton, it's a bomb!",
"JCDenton, it's a bomb!",
null
);
action.formatMentions({
return action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("TheJCDenton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">"+
"TheJCDenton</a>, it's a bomb!"
);
});
expect(action.text).toEqual("JCDenton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a>, it's a bomb!"
);
});
it("should highlight a user, regardless of case", () => {
let action = new MatrixAction(
Expand All @@ -32,33 +53,40 @@ describe("MatrixAction", function() {
"JCDenton, it's a bomb!",
null
);
action.formatMentions({
return action.formatMentions({
"jcdenton": "@jc.denton:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("TheJCDenton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">"+
"TheJCDenton</a>, it's a bomb!"
);
});
expect(action.text).toEqual("JCDenton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">jcdenton</a>, it's a bomb!"
);

});
it("should highlight a user, with plain text", () => {
let action = new MatrixAction("message", "JCDenton, it's a bomb!");
action.formatMentions({
return action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("TheJCDenton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">"+
"TheJCDenton</a>, it's a bomb!"
);
});
expect(action.text).toEqual("JCDenton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a>, it's a bomb!"
);
});
it("should highlight a user, with weird characters", () => {
let action = new MatrixAction("message", "`||JCDenton[m], it's a bomb!");
action.formatMentions({
return action.formatMentions({
"`||JCDenton[m]": "@jc.denton:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("TheJCDenton, it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">"+
"TheJCDenton</a>, it's a bomb!"
);
});
expect(action.text).toEqual("`||JCDenton[m], it's a bomb!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">`||JCDenton[m]</a>, it's a bomb!"
);
});
it("should highlight multiple users", () => {
let action = new MatrixAction(
Expand All @@ -67,15 +95,17 @@ describe("MatrixAction", function() {
"JCDenton is sent to assassinate PaulDenton",
null
);
action.formatMentions({
return action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov",
"PaulDenton": "@paul.denton:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("TheJCDenton is sent to assassinate ThePaulDenton");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">TheJCDenton</a> is sent" +
" to assassinate <a href=\"https://matrix.to/#/@paul.denton:unatco.gov\">" +
"ThePaulDenton</a>"
);
});
expect(action.text).toEqual("JCDenton is sent to assassinate PaulDenton");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a> is sent" +
" to assassinate <a href=\"https://matrix.to/#/@paul.denton:unatco.gov\">PaulDenton</a>"
);
});
it("should highlight multiple mentions of the same user", () => {
let action = new MatrixAction(
Expand All @@ -84,13 +114,63 @@ describe("MatrixAction", function() {
"JCDenton, meet JCDenton",
null
);
action.formatMentions({
return action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("TheJCDenton, meet TheJCDenton");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">TheJCDenton</a>," +
" meet <a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">TheJCDenton</a>"
);
});
expect(action.text).toEqual("JCDenton, meet JCDenton");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a>," +
" meet <a href=\"https://matrix.to/#/@jc.denton:unatco.gov\">JCDenton</a>"
});
it("should not highlight mentions in a URL with www.", () => {
let action = new MatrixAction(
"message",
"Go to http://www.JCDenton.com",
"Go to <a href='http://www.JCDenton.com'>my website</a>",
null
);
return action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("Go to http://www.JCDenton.com");
expect(action.htmlText).toEqual(
"Go to <a href='http://www.JCDenton.com'>my website</a>"
);
});
});
it("should not highlight mentions in a URL with http://", () => {
let action = new MatrixAction(
"message",
"Go to http://JCDenton.com",
"Go to <a href='http://JCDenton.com'>my website</a>",
null
);
return action.formatMentions({
"JCDenton": "@jc.denton:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("Go to http://JCDenton.com");
expect(action.htmlText).toEqual(
"Go to <a href='http://JCDenton.com'>my website</a>"
);
});
});
it("should fallback to userIds", () => {
let action = new MatrixAction(
"message",
"AnnaNavarre: The machine would not make a mistake!",
"AnnaNavarre: The machine would not make a mistake!",
null
);
return action.formatMentions({
"AnnaNavarre": "@anna.navarre:unatco.gov"
}, FakeIntent).then(() => {
expect(action.text).toEqual("anna.navarre: The machine would not make a mistake!");
expect(action.htmlText).toEqual(
"<a href=\"https://matrix.to/#/@anna.navarre:unatco.gov\">"+
"anna.navarre</a>: The machine would not make a mistake!"
);
});
});
});

0 comments on commit 4789667

Please sign in to comment.