From 3189bdba2a49368dce464202c86d270f427e2eed Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 16 Jan 2025 17:26:58 +0000 Subject: [PATCH] Fix displayname calculation around RTL / unhomoglyth. (#2953) --- src/state/CallViewModel.test.ts | 5 ++++- src/utils/displayname.test.ts | 22 +++++++++++++--------- src/utils/displayname.ts | 10 ++++++---- src/utils/test-fixtures.ts | 2 +- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 78a0d7ce..42c16049 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -886,7 +886,10 @@ test("should disambiguate users with invisible characters", () => { b: new Map([ [carolId, carol.userId], [bobId, `Bob (${bob.userId})`], - [bobZeroWidthSpaceId, `Bob (${bobZeroWidthSpace.userId})`], + [ + bobZeroWidthSpaceId, + `${bobZeroWidthSpace.rawDisplayName} (${bobZeroWidthSpace.userId})`, + ], ]), }, ); diff --git a/src/utils/displayname.test.ts b/src/utils/displayname.test.ts index fce473fa..df29a5dc 100644 --- a/src/utils/displayname.test.ts +++ b/src/utils/displayname.test.ts @@ -97,21 +97,25 @@ describe("calculateDisplayName", () => { test.for<[{ rawDisplayName?: string; userId: string }, boolean, string]>([ [alice, false, alice.rawDisplayName], [alice, true, `${alice.rawDisplayName} (${alice.userId})`], - [alice, false, alice.rawDisplayName], + // Empty strings and zero width strings that are effectively empty are resolved as userIds [{ rawDisplayName: "", userId: alice.userId }, false, alice.userId], - [ - { rawDisplayName: alice.userId, userId: alice.userId }, - false, - alice.userId, - ], - [bobZeroWidthSpace, false, "Bob"], [ { rawDisplayName: "\u200b\u200b\u200b", userId: alice.userId }, false, alice.userId, ], - [daveRTL, false, "evaD"], - [daveRTL, true, `evaD (${daveRTL.userId})`], + [ + { rawDisplayName: alice.userId, userId: alice.userId }, + false, + alice.userId, + ], + // Zero width strings are kept intact + [bobZeroWidthSpace, false, bobZeroWidthSpace.rawDisplayName], + // Directional characters are stripped. + [daveRTL, false, daveRTL.rawDisplayName.slice(1)], + [daveRTL, true, `${daveRTL.rawDisplayName.slice(1)} (${daveRTL.userId})`], + // Ensure we do NOT unhomoglyth + [{ ...alice, rawDisplayName: "alice m" }, false, "alice m"], ])("correctly calculates displayname", ([member, disambiguate, result]) => expect(calculateDisplayName(member, disambiguate)).toEqual(result), ); diff --git a/src/utils/displayname.ts b/src/utils/displayname.ts index 63b54ebc..1a3ac930 100644 --- a/src/utils/displayname.ts +++ b/src/utils/displayname.ts @@ -45,7 +45,11 @@ export function shouldDisambiguate( // NOTE: We *should* have a room member for everyone. .filter((m) => !!m) .filter((m) => m.userId !== userId) - .some((m) => calculateDisplayName(m, false) === strippedDisplayName) + .some( + (m) => + removeHiddenChars(calculateDisplayName(m, false)) === + strippedDisplayName, + ) ); } @@ -56,9 +60,7 @@ export function calculateDisplayName( const { rawDisplayName: displayName, userId } = member; if (!displayName || displayName === userId) return userId; - const resultDisplayname = removeDirectionOverrideChars( - removeHiddenChars(displayName), - ); + const resultDisplayname = removeDirectionOverrideChars(displayName); if (disambiguate) return resultDisplayname + " (" + userId + ")"; diff --git a/src/utils/test-fixtures.ts b/src/utils/test-fixtures.ts index 1172d111..95fb4fbd 100644 --- a/src/utils/test-fixtures.ts +++ b/src/utils/test-fixtures.ts @@ -56,6 +56,6 @@ export const bobZeroWidthSpaceId = `${bobZeroWidthSpace.userId}:${bobZeroWidthSp export const daveRTLRtcMember = mockRtcMembership("@dave2:example.org", "DDDD"); export const daveRTL = mockMatrixRoomMember(daveRTLRtcMember, { - rawDisplayName: "\u200fevaD", + rawDisplayName: "\u202eevaD", }); export const daveRTLId = `${daveRTL.userId}:${daveRTLRtcMember.deviceId}`;