Fix displayname calculation around RTL / unhomoglyth. (#2953)

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})`,
+              ],
             ]),
           },
         );

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),
   );

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(
+  const resultDisplayname = removeDirectionOverrideChars(displayName);
-    removeHiddenChars(displayName),
-  );
 
   if (disambiguate) return resultDisplayname + " (" + userId + ")";
 

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}`;