From b79343d5b09d14e68c83ae4bca8447932ff730a5 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:38:53 +0100 Subject: [PATCH 1/7] Improve logging in SessionLock (#133) some more diagnostics for this, to help resolve https://github.com/element-hq/element-desktop/issues/1495 --- src/utils/SessionLock.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/utils/SessionLock.ts b/src/utils/SessionLock.ts index 6a1c65584d..e3d9d5ecc8 100644 --- a/src/utils/SessionLock.ts +++ b/src/utils/SessionLock.ts @@ -66,15 +66,29 @@ export const SESSION_LOCK_CONSTANTS = { * @returns true if any instance is currently active */ export function checkSessionLockFree(): boolean { + const prefixedLogger = logger.getChild(`checkSessionLockFree`); + const lastPingTime = window.localStorage.getItem(SESSION_LOCK_CONSTANTS.STORAGE_ITEM_PING); if (lastPingTime === null) { // no other holder + prefixedLogger.info("No other session has the lock"); return true; } + const lockHolder = window.localStorage.getItem(SESSION_LOCK_CONSTANTS.STORAGE_ITEM_OWNER); + // see if it has expired const timeAgo = Date.now() - parseInt(lastPingTime); - return timeAgo > SESSION_LOCK_CONSTANTS.LOCK_EXPIRY_TIME_MS; + + const remaining = SESSION_LOCK_CONSTANTS.LOCK_EXPIRY_TIME_MS - timeAgo; + if (remaining <= 0) { + // another session claimed the lock, but it is stale. + prefixedLogger.info(`Last ping (from ${lockHolder}) was ${timeAgo}ms ago: lock is free`); + return true; + } + + prefixedLogger.info(`Last ping (from ${lockHolder}) was ${timeAgo}ms ago: lock is taken`); + return false; } /** @@ -95,7 +109,7 @@ export async function getSessionLock(onNewInstance: () => Promise): Promis /** unique ID for this session */ const sessionIdentifier = uuidv4(); - const prefixedLogger = logger.withPrefix(`getSessionLock[${sessionIdentifier}]`); + const prefixedLogger = logger.getChild(`getSessionLock[${sessionIdentifier}]`); /** The ID of our regular task to service the lock. * @@ -133,7 +147,7 @@ export async function getSessionLock(onNewInstance: () => Promise): Promis return 0; } - prefixedLogger.info(`Last ping (from ${lockHolder}) was ${timeAgo}ms ago, waiting`); + prefixedLogger.info(`Last ping (from ${lockHolder}) was ${timeAgo}ms ago, waiting ${remaining}ms`); return remaining; } From a2cee6bbb42a1caba88e3dc9d55f3f4340ced2c0 Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 9 Oct 2024 17:34:42 +0100 Subject: [PATCH 2/7] Fix vertical overflow on the mobile register screen (#137) * Fix vertical overflow on the mobile register screen * Change to border-box to include padding in height so the register button can be scrolled fully in to view. --- res/css/structures/auth/_MobileRegistration.pcss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/res/css/structures/auth/_MobileRegistration.pcss b/res/css/structures/auth/_MobileRegistration.pcss index d50ff8dc1f..e2ba1cba28 100644 --- a/res/css/structures/auth/_MobileRegistration.pcss +++ b/res/css/structures/auth/_MobileRegistration.pcss @@ -7,4 +7,7 @@ Please see LICENSE files in the repository root for full details. .mx_MobileRegister_body { padding: 32px; + height: 100vh; + overflow-y: auto; + box-sizing: border-box; } From 5f113c4db0c6706ffbdc240a55087db014e32d43 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 9 Oct 2024 23:44:10 +0100 Subject: [PATCH 3/7] Fix bell icons on room list hover being black squares (#135) * Fix bell icons on room list hover being black squares The EchoStore wasn't being set up and therefore missed the client being injected. Patch from @t3chguy. * Fix tests ...by mocking out createClient so it doesn't try to start a real client. * More mocks * Don't need this anymore either --- src/stores/ReadyWatchingStore.ts | 2 ++ test/MatrixClientPeg-test.ts | 20 +++++++++----------- test/test-utils/test-utils.ts | 7 ++++++- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/stores/ReadyWatchingStore.ts b/src/stores/ReadyWatchingStore.ts index 393b6fea11..a2e8c278f0 100644 --- a/src/stores/ReadyWatchingStore.ts +++ b/src/stores/ReadyWatchingStore.ts @@ -29,6 +29,8 @@ export abstract class ReadyWatchingStore extends EventEmitter implements IDestro super(); this.dispatcherRef = this.dispatcher.register(this.onAction); + + ReadyWatchingStore.instances.push(this); } public get matrixClient(): MatrixClient | null { diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 9634a6a54f..4d7f769d04 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -6,8 +6,8 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ +import * as MatrixJs from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; -import fetchMockJest from "fetch-mock-jest"; import { advanceDateAndTime, stubClient } from "./test-utils"; import { IMatrixClientPeg, MatrixClientPeg as peg } from "../src/MatrixClientPeg"; @@ -19,9 +19,14 @@ jest.useFakeTimers(); const PegClass = Object.getPrototypeOf(peg).constructor; describe("MatrixClientPeg", () => { + let mockClient: MatrixJs.MatrixClient; + beforeEach(() => { // stub out Logger.log which gets called a lot and clutters up the test output jest.spyOn(logger, "log").mockImplementation(() => {}); + + mockClient = stubClient(); + jest.spyOn(MatrixJs, "createClient").mockReturnValue(mockClient); }); afterEach(() => { @@ -33,7 +38,6 @@ describe("MatrixClientPeg", () => { }); it("setJustRegisteredUserId", () => { - stubClient(); (peg as any).matrixClient = peg.get(); peg.setJustRegisteredUserId("@userId:matrix.org"); expect(peg.safeGet().credentials.userId).toBe("@userId:matrix.org"); @@ -52,7 +56,6 @@ describe("MatrixClientPeg", () => { }); it("setJustRegisteredUserId(null)", () => { - stubClient(); (peg as any).matrixClient = peg.get(); peg.setJustRegisteredUserId(null); expect(peg.currentUserIsJustRegistered()).toBe(false); @@ -71,7 +74,6 @@ describe("MatrixClientPeg", () => { beforeEach(() => { // instantiate a MatrixClientPegClass instance, with a new MatrixClient testPeg = new PegClass(); - fetchMockJest.get("http://example.com/_matrix/client/versions", {}); testPeg.replaceUsingCreds({ accessToken: "SEKRET", homeserverUrl: "http://example.com", @@ -83,13 +85,10 @@ describe("MatrixClientPeg", () => { it("should initialise the rust crypto library by default", async () => { const mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); - const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockResolvedValue(undefined); - const mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined); - const cryptoStoreKey = new Uint8Array([1, 2, 3, 4]); await testPeg.start({ rustCryptoStoreKey: cryptoStoreKey }); - expect(mockInitCrypto).not.toHaveBeenCalled(); - expect(mockInitRustCrypto).toHaveBeenCalledWith({ storageKey: cryptoStoreKey }); + expect(mockClient.initCrypto).not.toHaveBeenCalled(); + expect(mockClient.initRustCrypto).toHaveBeenCalledWith({ storageKey: cryptoStoreKey }); // we should have stashed the setting in the settings store expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, true); @@ -97,10 +96,9 @@ describe("MatrixClientPeg", () => { it("Should migrate existing login", async () => { const mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); - const mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined); await testPeg.start(); - expect(mockInitRustCrypto).toHaveBeenCalledTimes(1); + expect(mockClient.initRustCrypto).toHaveBeenCalledTimes(1); // we should have stashed the setting in the settings store expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, true); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index ebfc6b221b..fe9d078f3a 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -86,13 +86,14 @@ export function createTestClient(): MatrixClient { let txnId = 1; const client = { + startClient: jest.fn(), getHomeserverUrl: jest.fn(), getIdentityServerUrl: jest.fn(), getDomain: jest.fn().mockReturnValue("matrix.org"), getUserId: jest.fn().mockReturnValue("@userId:matrix.org"), getSafeUserId: jest.fn().mockReturnValue("@userId:matrix.org"), getUserIdLocalpart: jest.fn().mockResolvedValue("userId"), - getUser: jest.fn().mockReturnValue({ on: jest.fn(), off: jest.fn() }), + getUser: jest.fn().mockReturnValue({ on: jest.fn(), off: jest.fn(), removeListener: jest.fn() }), getDevice: jest.fn(), getDeviceId: jest.fn().mockReturnValue("ABCDEFGHI"), getStoredCrossSigningForUser: jest.fn(), @@ -133,6 +134,8 @@ export function createTestClient(): MatrixClient { getVerificationRequestsToDeviceInProgress: jest.fn().mockReturnValue([]), setDeviceIsolationMode: jest.fn(), }), + initCrypto: jest.fn(), + initRustCrypto: jest.fn(), getPushActionsForEvent: jest.fn(), getRoom: jest.fn().mockImplementation((roomId) => mkStubRoom(roomId, "My room", client)), @@ -180,6 +183,7 @@ export function createTestClient(): MatrixClient { getSyncState: jest.fn().mockReturnValue("SYNCING"), generateClientSecret: () => "t35tcl1Ent5ECr3T", isGuest: jest.fn().mockReturnValue(false), + setGuest: jest.fn(), getRoomHierarchy: jest.fn().mockReturnValue({ rooms: [], }), @@ -277,6 +281,7 @@ export function createTestClient(): MatrixClient { isFallbackICEServerAllowed: jest.fn().mockReturnValue(false), getAuthIssuer: jest.fn(), getOrCreateFilter: jest.fn(), + setNotifTimelineSet: jest.fn(), } as unknown as MatrixClient; client.reEmitter = new ReEmitter(client); From 4e5cf1b72068c5efd37c6064b9eb8dfcd2e2841c Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 10 Oct 2024 09:30:08 +0200 Subject: [PATCH 4/7] Add aria-label to e2e icon (#136) --- src/components/views/rooms/EventTile.tsx | 2 +- src/i18n/strings/en_EN.json | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 81791bdcce..0089773dc8 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -1519,7 +1519,7 @@ class E2ePadlock extends React.Component { // https://github.com/element-hq/compound/issues/294 return ( -
+
); } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 8b43b71ac1..74265c1939 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3272,6 +3272,7 @@ "download_action_downloading": "Downloading", "download_failed": "Download failed", "download_failed_description": "An error occurred while downloading this file", + "e2e_state": "State of the end-to-end encryption", "edits": { "tooltip_label": "Edited at %(date)s. Click to view edits.", "tooltip_sub": "Click to view edits", From 3a595567498a2550ebefa206a4482c30fe8cb6d4 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 10 Oct 2024 15:08:43 +0100 Subject: [PATCH 5/7] Revert #124 and #135 (#139) This seems to be causing a lot of weirdness, presumably because there's another missing thing like in #135, but I don't know what it might be and it feels like it might take a while to find. Backing these changes out to fix develop while we sort it out. Fixes https://github.com/element-hq/element-web/issues/28179 --- src/MatrixClientPeg.ts | 2 - src/stores/AsyncStoreWithClient.ts | 9 +---- src/stores/AutoRageshakeStore.ts | 4 +- src/stores/BreadcrumbsStore.ts | 4 +- src/stores/CallStore.ts | 1 + src/stores/ModalWidgetStore.ts | 1 + src/stores/OwnBeaconStore.ts | 1 + src/stores/OwnProfileStore.ts | 1 + src/stores/ReadyWatchingStore.ts | 39 ++++++------------- src/stores/VoiceRecordingStore.ts | 1 + src/stores/WidgetStore.ts | 1 + src/stores/local-echo/EchoStore.ts | 1 + .../RoomNotificationStateStore.ts | 1 + src/stores/right-panel/RightPanelStore.ts | 1 + src/stores/room-list/MessagePreviewStore.ts | 6 ++- src/stores/room-list/RoomListLayoutStore.ts | 1 + src/stores/room-list/RoomListStore.ts | 2 + src/stores/spaces/SpaceStore.ts | 6 +-- src/stores/widgets/WidgetLayoutStore.ts | 1 + src/stores/widgets/WidgetMessagingStore.ts | 1 + test/MatrixClientPeg-test.ts | 20 +++++----- test/stores/AutoRageshakeStore-test.ts | 2 +- test/stores/OwnProfileStore-test.ts | 6 +-- test/stores/SpaceStore-test.ts | 4 +- test/stores/VoiceRecordingStore-test.ts | 2 +- test/stores/WidgetLayoutStore-test.ts | 8 ++-- .../room-list/MessagePreviewStore-test.ts | 2 +- test/stores/room-list/RoomListStore-test.ts | 15 +++---- .../room-list/SlidingRoomListStore-test.ts | 16 ++++---- test/test-utils/test-utils.ts | 7 +--- 30 files changed, 78 insertions(+), 88 deletions(-) diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index e5ca4464ba..0ba2ec65d7 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -43,7 +43,6 @@ import { formatList } from "./utils/FormattingUtils"; import SdkConfig from "./SdkConfig"; import { Features } from "./settings/Settings"; import { setDeviceIsolationMode } from "./settings/controllers/DeviceIsolationModeController.ts"; -import { ReadyWatchingStore } from "./stores/ReadyWatchingStore.ts"; export interface IMatrixClientCreds { homeserverUrl: string; @@ -310,7 +309,6 @@ class MatrixClientPegClass implements IMatrixClientPeg { MatrixActionCreators.start(this.matrixClient); MatrixClientBackedSettingsHandler.matrixClient = this.matrixClient; MatrixClientBackedController.matrixClient = this.matrixClient; - ReadyWatchingStore.matrixClient = this.matrixClient; return opts; } diff --git a/src/stores/AsyncStoreWithClient.ts b/src/stores/AsyncStoreWithClient.ts index d9facfc51a..7567eac9ac 100644 --- a/src/stores/AsyncStoreWithClient.ts +++ b/src/stores/AsyncStoreWithClient.ts @@ -36,13 +36,8 @@ export abstract class AsyncStoreWithClient extends AsyncStore< })(dispatcher); } - protected async start(matrixClient: MatrixClient | null): Promise { - await this.readyStore.start(matrixClient); - } - - // XXX: This method is intended only for use in tests. - public async useUnitTestClient(cli: MatrixClient): Promise { - await this.readyStore.useUnitTestClient(cli); + public async start(): Promise { + await this.readyStore.start(); } public get matrixClient(): MatrixClient | null { diff --git a/src/stores/AutoRageshakeStore.ts b/src/stores/AutoRageshakeStore.ts index e83baf5a9c..284c3e24a4 100644 --- a/src/stores/AutoRageshakeStore.ts +++ b/src/stores/AutoRageshakeStore.ts @@ -46,7 +46,9 @@ interface IState { */ export default class AutoRageshakeStore extends AsyncStoreWithClient { private static readonly internalInstance = (() => { - return new AutoRageshakeStore(); + const instance = new AutoRageshakeStore(); + instance.start(); + return instance; })(); private constructor() { diff --git a/src/stores/BreadcrumbsStore.ts b/src/stores/BreadcrumbsStore.ts index 546f1e63ae..9859f24015 100644 --- a/src/stores/BreadcrumbsStore.ts +++ b/src/stores/BreadcrumbsStore.ts @@ -30,7 +30,9 @@ interface IState { export class BreadcrumbsStore extends AsyncStoreWithClient { private static readonly internalInstance = (() => { - return new BreadcrumbsStore(); + const instance = new BreadcrumbsStore(); + instance.start(); + return instance; })(); private waitingRooms: { roomId: string; addedTs: number }[] = []; diff --git a/src/stores/CallStore.ts b/src/stores/CallStore.ts index f0120ee6a6..115a56aced 100644 --- a/src/stores/CallStore.ts +++ b/src/stores/CallStore.ts @@ -31,6 +31,7 @@ export class CallStore extends AsyncStoreWithClient<{}> { public static get instance(): CallStore { if (!this._instance) { this._instance = new CallStore(); + this._instance.start(); } return this._instance; } diff --git a/src/stores/ModalWidgetStore.ts b/src/stores/ModalWidgetStore.ts index aa4d5d1466..59437db403 100644 --- a/src/stores/ModalWidgetStore.ts +++ b/src/stores/ModalWidgetStore.ts @@ -24,6 +24,7 @@ interface IState { export class ModalWidgetStore extends AsyncStoreWithClient { private static readonly internalInstance = (() => { const instance = new ModalWidgetStore(); + instance.start(); return instance; })(); private modalInstance: IHandle | null = null; diff --git a/src/stores/OwnBeaconStore.ts b/src/stores/OwnBeaconStore.ts index 4240d7b7df..f60dae07fe 100644 --- a/src/stores/OwnBeaconStore.ts +++ b/src/stores/OwnBeaconStore.ts @@ -87,6 +87,7 @@ const getLocallyCreatedBeaconEventIds = (): string[] => { export class OwnBeaconStore extends AsyncStoreWithClient { private static readonly internalInstance = (() => { const instance = new OwnBeaconStore(); + instance.start(); return instance; })(); // users beacons, keyed by event type diff --git a/src/stores/OwnProfileStore.ts b/src/stores/OwnProfileStore.ts index f0b94df553..be8c72aa23 100644 --- a/src/stores/OwnProfileStore.ts +++ b/src/stores/OwnProfileStore.ts @@ -28,6 +28,7 @@ const KEY_AVATAR_URL = "mx_profile_avatar_url"; export class OwnProfileStore extends AsyncStoreWithClient { private static readonly internalInstance = (() => { const instance = new OwnProfileStore(); + instance.start(); return instance; })(); diff --git a/src/stores/ReadyWatchingStore.ts b/src/stores/ReadyWatchingStore.ts index a2e8c278f0..a46a09899a 100644 --- a/src/stores/ReadyWatchingStore.ts +++ b/src/stores/ReadyWatchingStore.ts @@ -9,42 +9,27 @@ import { MatrixClient, SyncState } from "matrix-js-sdk/src/matrix"; import { EventEmitter } from "events"; +import { MatrixClientPeg } from "../MatrixClientPeg"; import { ActionPayload } from "../dispatcher/payloads"; import { IDestroyable } from "../utils/IDestroyable"; import { Action } from "../dispatcher/actions"; import { MatrixDispatcher } from "../dispatcher/dispatcher"; export abstract class ReadyWatchingStore extends EventEmitter implements IDestroyable { - private static instances: ReadyWatchingStore[] = []; - protected _matrixClient: MatrixClient | null = null; + protected matrixClient: MatrixClient | null = null; private dispatcherRef: string | null = null; - public static set matrixClient(client: MatrixClient) { - for (const instance of ReadyWatchingStore.instances) { - instance.start(client); - } - } - public constructor(protected readonly dispatcher: MatrixDispatcher) { super(); + } + public async start(): Promise { this.dispatcherRef = this.dispatcher.register(this.onAction); - ReadyWatchingStore.instances.push(this); - } - - public get matrixClient(): MatrixClient | null { - return this._matrixClient; - } - - public async start(matrixClient: MatrixClient | null): Promise { - const oldClient = this._matrixClient; - this._matrixClient = matrixClient; - - if (oldClient !== matrixClient) { - await this.onNotReady(); - } + // MatrixClientPeg can be undefined in tests because of circular dependencies with other stores + const matrixClient = MatrixClientPeg?.get(); if (matrixClient) { + this.matrixClient = matrixClient; await this.onReady(); } } @@ -53,10 +38,8 @@ export abstract class ReadyWatchingStore extends EventEmitter implements IDestro return this.matrixClient; // for external readonly access } - // XXX: This method is intended only for use in tests. - public async useUnitTestClient(cli: MatrixClient): Promise { - this._matrixClient = cli; - await this.onReady(); + public useUnitTestClient(cli: MatrixClient): void { + this.matrixClient = cli; } public destroy(): void { @@ -91,13 +74,13 @@ export abstract class ReadyWatchingStore extends EventEmitter implements IDestro if (this.matrixClient) { await this.onNotReady(); } - this._matrixClient = payload.matrixClient; + this.matrixClient = payload.matrixClient; await this.onReady(); } } else if (payload.action === "on_client_not_viable" || payload.action === Action.OnLoggedOut) { if (this.matrixClient) { await this.onNotReady(); - this._matrixClient = null; + this.matrixClient = null; } } }; diff --git a/src/stores/VoiceRecordingStore.ts b/src/stores/VoiceRecordingStore.ts index 7a9e3601cf..7cbb8ac120 100644 --- a/src/stores/VoiceRecordingStore.ts +++ b/src/stores/VoiceRecordingStore.ts @@ -30,6 +30,7 @@ export class VoiceRecordingStore extends AsyncStoreWithClient { public static get instance(): VoiceRecordingStore { if (!this.internalInstance) { this.internalInstance = new VoiceRecordingStore(); + this.internalInstance.start(); } return this.internalInstance; } diff --git a/src/stores/WidgetStore.ts b/src/stores/WidgetStore.ts index 071cf8bde9..cfb92360a0 100644 --- a/src/stores/WidgetStore.ts +++ b/src/stores/WidgetStore.ts @@ -45,6 +45,7 @@ interface IRoomWidgets { export default class WidgetStore extends AsyncStoreWithClient { private static readonly internalInstance = (() => { const instance = new WidgetStore(); + instance.start(); return instance; })(); diff --git a/src/stores/local-echo/EchoStore.ts b/src/stores/local-echo/EchoStore.ts index 41c92941ea..956eacb929 100644 --- a/src/stores/local-echo/EchoStore.ts +++ b/src/stores/local-echo/EchoStore.ts @@ -38,6 +38,7 @@ export class EchoStore extends AsyncStoreWithClient { public static get instance(): EchoStore { if (!this._instance) { this._instance = new EchoStore(); + this._instance.start(); } return this._instance; } diff --git a/src/stores/notifications/RoomNotificationStateStore.ts b/src/stores/notifications/RoomNotificationStateStore.ts index ebe9129989..87fb276c10 100644 --- a/src/stores/notifications/RoomNotificationStateStore.ts +++ b/src/stores/notifications/RoomNotificationStateStore.ts @@ -26,6 +26,7 @@ export const UPDATE_STATUS_INDICATOR = Symbol("update-status-indicator"); export class RoomNotificationStateStore extends AsyncStoreWithClient { private static readonly internalInstance = (() => { const instance = new RoomNotificationStateStore(); + instance.start(); return instance; })(); private roomMap = new Map(); diff --git a/src/stores/right-panel/RightPanelStore.ts b/src/stores/right-panel/RightPanelStore.ts index 4415852758..43a36e91b8 100644 --- a/src/stores/right-panel/RightPanelStore.ts +++ b/src/stores/right-panel/RightPanelStore.ts @@ -403,6 +403,7 @@ export default class RightPanelStore extends ReadyWatchingStore { public static get instance(): RightPanelStore { if (!this.internalInstance) { this.internalInstance = new RightPanelStore(); + this.internalInstance.start(); } return this.internalInstance; } diff --git a/src/stores/room-list/MessagePreviewStore.ts b/src/stores/room-list/MessagePreviewStore.ts index 1462826fe4..e0e06ec980 100644 --- a/src/stores/room-list/MessagePreviewStore.ts +++ b/src/stores/room-list/MessagePreviewStore.ts @@ -124,7 +124,11 @@ const mkMessagePreview = (text: string, event: MatrixEvent): MessagePreview => { }; export class MessagePreviewStore extends AsyncStoreWithClient { - private static readonly internalInstance = (() => new MessagePreviewStore())(); + private static readonly internalInstance = (() => { + const instance = new MessagePreviewStore(); + instance.start(); + return instance; + })(); /** * @internal Public for test only diff --git a/src/stores/room-list/RoomListLayoutStore.ts b/src/stores/room-list/RoomListLayoutStore.ts index d305bacffb..ea85860554 100644 --- a/src/stores/room-list/RoomListLayoutStore.ts +++ b/src/stores/room-list/RoomListLayoutStore.ts @@ -28,6 +28,7 @@ export default class RoomListLayoutStore extends AsyncStoreWithClient { public static get instance(): RoomListLayoutStore { if (!this.internalInstance) { this.internalInstance = new RoomListLayoutStore(); + this.internalInstance.start(); } return RoomListLayoutStore.internalInstance; } diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index bb7cf15e8e..53377e0a01 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -643,9 +643,11 @@ export default class RoomListStore { if (SettingsStore.getValue("feature_sliding_sync")) { logger.info("using SlidingRoomListStoreClass"); const instance = new SlidingRoomListStoreClass(defaultDispatcher, SdkContextClass.instance); + instance.start(); RoomListStore.internalInstance = instance; } else { const instance = new RoomListStoreClass(defaultDispatcher); + instance.start(); RoomListStore.internalInstance = instance; } } diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 1c3afeaf85..90358f3310 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -17,7 +17,6 @@ import { MatrixEvent, ClientEvent, ISendEventResponse, - MatrixClient, } from "matrix-js-sdk/src/matrix"; import { KnownMembership } from "matrix-js-sdk/src/types"; import { logger } from "matrix-js-sdk/src/logger"; @@ -1398,6 +1397,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { export default class SpaceStore { private static readonly internalInstance = (() => { const instance = new SpaceStoreClass(); + instance.start(); return instance; })(); @@ -1408,9 +1408,9 @@ export default class SpaceStore { /** * @internal for test only */ - public static testInstance(client: MatrixClient): SpaceStoreClass { + public static testInstance(): SpaceStoreClass { const store = new SpaceStoreClass(); - store.useUnitTestClient(client); + store.start(); return store; } } diff --git a/src/stores/widgets/WidgetLayoutStore.ts b/src/stores/widgets/WidgetLayoutStore.ts index 00b27a1aa1..cefbee0f6b 100644 --- a/src/stores/widgets/WidgetLayoutStore.ts +++ b/src/stores/widgets/WidgetLayoutStore.ts @@ -60,6 +60,7 @@ export class WidgetLayoutStore extends ReadyWatchingStore { public static get instance(): WidgetLayoutStore { if (!this.internalInstance) { this.internalInstance = new WidgetLayoutStore(); + this.internalInstance.start(); } return this.internalInstance; } diff --git a/src/stores/widgets/WidgetMessagingStore.ts b/src/stores/widgets/WidgetMessagingStore.ts index 404049ab36..0388aeacb4 100644 --- a/src/stores/widgets/WidgetMessagingStore.ts +++ b/src/stores/widgets/WidgetMessagingStore.ts @@ -27,6 +27,7 @@ export enum WidgetMessagingStoreEvent { export class WidgetMessagingStore extends AsyncStoreWithClient<{}> { private static readonly internalInstance = (() => { const instance = new WidgetMessagingStore(); + instance.start(); return instance; })(); diff --git a/test/MatrixClientPeg-test.ts b/test/MatrixClientPeg-test.ts index 4d7f769d04..9634a6a54f 100644 --- a/test/MatrixClientPeg-test.ts +++ b/test/MatrixClientPeg-test.ts @@ -6,8 +6,8 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ -import * as MatrixJs from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; +import fetchMockJest from "fetch-mock-jest"; import { advanceDateAndTime, stubClient } from "./test-utils"; import { IMatrixClientPeg, MatrixClientPeg as peg } from "../src/MatrixClientPeg"; @@ -19,14 +19,9 @@ jest.useFakeTimers(); const PegClass = Object.getPrototypeOf(peg).constructor; describe("MatrixClientPeg", () => { - let mockClient: MatrixJs.MatrixClient; - beforeEach(() => { // stub out Logger.log which gets called a lot and clutters up the test output jest.spyOn(logger, "log").mockImplementation(() => {}); - - mockClient = stubClient(); - jest.spyOn(MatrixJs, "createClient").mockReturnValue(mockClient); }); afterEach(() => { @@ -38,6 +33,7 @@ describe("MatrixClientPeg", () => { }); it("setJustRegisteredUserId", () => { + stubClient(); (peg as any).matrixClient = peg.get(); peg.setJustRegisteredUserId("@userId:matrix.org"); expect(peg.safeGet().credentials.userId).toBe("@userId:matrix.org"); @@ -56,6 +52,7 @@ describe("MatrixClientPeg", () => { }); it("setJustRegisteredUserId(null)", () => { + stubClient(); (peg as any).matrixClient = peg.get(); peg.setJustRegisteredUserId(null); expect(peg.currentUserIsJustRegistered()).toBe(false); @@ -74,6 +71,7 @@ describe("MatrixClientPeg", () => { beforeEach(() => { // instantiate a MatrixClientPegClass instance, with a new MatrixClient testPeg = new PegClass(); + fetchMockJest.get("http://example.com/_matrix/client/versions", {}); testPeg.replaceUsingCreds({ accessToken: "SEKRET", homeserverUrl: "http://example.com", @@ -85,10 +83,13 @@ describe("MatrixClientPeg", () => { it("should initialise the rust crypto library by default", async () => { const mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); + const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockResolvedValue(undefined); + const mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined); + const cryptoStoreKey = new Uint8Array([1, 2, 3, 4]); await testPeg.start({ rustCryptoStoreKey: cryptoStoreKey }); - expect(mockClient.initCrypto).not.toHaveBeenCalled(); - expect(mockClient.initRustCrypto).toHaveBeenCalledWith({ storageKey: cryptoStoreKey }); + expect(mockInitCrypto).not.toHaveBeenCalled(); + expect(mockInitRustCrypto).toHaveBeenCalledWith({ storageKey: cryptoStoreKey }); // we should have stashed the setting in the settings store expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, true); @@ -96,9 +97,10 @@ describe("MatrixClientPeg", () => { it("Should migrate existing login", async () => { const mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined); + const mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined); await testPeg.start(); - expect(mockClient.initRustCrypto).toHaveBeenCalledTimes(1); + expect(mockInitRustCrypto).toHaveBeenCalledTimes(1); // we should have stashed the setting in the settings store expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, true); diff --git a/test/stores/AutoRageshakeStore-test.ts b/test/stores/AutoRageshakeStore-test.ts index 7ed317fc81..f321e909a4 100644 --- a/test/stores/AutoRageshakeStore-test.ts +++ b/test/stores/AutoRageshakeStore-test.ts @@ -47,7 +47,7 @@ describe("AutoRageshakeStore", () => { // @ts-ignore bypass private ctor for tests autoRageshakeStore = new AutoRageshakeStore(); - autoRageshakeStore.useUnitTestClient(client); + autoRageshakeStore.start(); utdEvent = mkEvent({ event: true, diff --git a/test/stores/OwnProfileStore-test.ts b/test/stores/OwnProfileStore-test.ts index 628706372d..0f4f2d325a 100644 --- a/test/stores/OwnProfileStore-test.ts +++ b/test/stores/OwnProfileStore-test.ts @@ -40,7 +40,7 @@ describe("OwnProfileStore", () => { displayname: "Display Name", avatar_url: "mxc://example.com/abc123", }); - await ownProfileStore.useUnitTestClient(client); + await ownProfileStore.start(); expect(onUpdate).toHaveBeenCalled(); expect(ownProfileStore.displayName).toBe("Display Name"); @@ -54,7 +54,7 @@ describe("OwnProfileStore", () => { errcode: "M_NOT_FOUND", }), ); - await ownProfileStore.useUnitTestClient(client); + await ownProfileStore.start(); expect(onUpdate).toHaveBeenCalled(); expect(ownProfileStore.displayName).toBe(client.getSafeUserId()); @@ -69,7 +69,7 @@ describe("OwnProfileStore", () => { }), ); try { - await ownProfileStore.useUnitTestClient(client); + await ownProfileStore.start(); } catch (ignore) {} expect(onUpdate).not.toHaveBeenCalled(); diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 53172f946a..35ec275c49 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -1428,7 +1428,7 @@ describe("SpaceStore", () => { it("passes that value in calls to getVisibleRooms during getSpaceFilteredRoomIds", () => { // Given a store - const store = SpaceStore.testInstance(client); + const store = SpaceStore.testInstance(); // When we ask for filtered room ids store.getSpaceFilteredRoomIds(MetaSpace.Home); @@ -1478,7 +1478,7 @@ describe("SpaceStore", () => { it("passes that value in calls to getVisibleRooms during getSpaceFilteredRoomIds", () => { // Given a store - const store = SpaceStore.testInstance(client); + const store = SpaceStore.testInstance(); // When we ask for filtered room ids store.getSpaceFilteredRoomIds(MetaSpace.Home); diff --git a/test/stores/VoiceRecordingStore-test.ts b/test/stores/VoiceRecordingStore-test.ts index 9af96914e8..6974e6063e 100644 --- a/test/stores/VoiceRecordingStore-test.ts +++ b/test/stores/VoiceRecordingStore-test.ts @@ -31,7 +31,7 @@ describe("VoiceRecordingStore", () => { const mkStore = (): VoiceRecordingStore => { const store = new VoiceRecordingStore(); - store.useUnitTestClient(stubClient); + store.start(); return store; }; diff --git a/test/stores/WidgetLayoutStore-test.ts b/test/stores/WidgetLayoutStore-test.ts index 4ac397a121..72418dda5f 100644 --- a/test/stores/WidgetLayoutStore-test.ts +++ b/test/stores/WidgetLayoutStore-test.ts @@ -167,7 +167,7 @@ describe("WidgetLayoutStore", () => { it("should recalculate all rooms when the client is ready", async () => { mocked(client.getVisibleRooms).mockReturnValue([mockRoom]); - await store.start(client); + await store.start(); expect(roomUpdateListener).toHaveBeenCalled(); expect(store.getContainerWidgets(mockRoom, Container.Top)).toEqual([]); @@ -243,7 +243,7 @@ describe("WidgetLayoutStore", () => { }); it("should copy the layout to the room", async () => { - await store.start(client); + await store.start(); store.recalculateRoom(mockRoom); store.moveToContainer(mockRoom, mockApps[0], Container.Top); store.copyLayoutToRoom(mockRoom); @@ -297,7 +297,7 @@ describe("WidgetLayoutStore", () => { mocked(client.getVisibleRooms).mockReturnValue([]); // @ts-ignore bypass private ctor for tests const store = new WidgetLayoutStore(); - await store.start(client); + await store.start(); expect(client.getVisibleRooms).toHaveBeenCalledWith(false); }); }); @@ -314,7 +314,7 @@ describe("WidgetLayoutStore", () => { mocked(client.getVisibleRooms).mockReturnValue([]); // @ts-ignore bypass private ctor for tests const store = new WidgetLayoutStore(); - await store.start(client); + await store.start(); expect(client.getVisibleRooms).toHaveBeenCalledWith(true); }); }); diff --git a/test/stores/room-list/MessagePreviewStore-test.ts b/test/stores/room-list/MessagePreviewStore-test.ts index 603f4713e9..976c822253 100644 --- a/test/stores/room-list/MessagePreviewStore-test.ts +++ b/test/stores/room-list/MessagePreviewStore-test.ts @@ -80,7 +80,7 @@ describe("MessagePreviewStore", () => { mocked(client.getRoom).mockReturnValue(room); store = MessagePreviewStore.testInstance(); - await store.useUnitTestClient(client); + await store.start(); await setupAsyncStoreWithClient(store, client); }); diff --git a/test/stores/room-list/RoomListStore-test.ts b/test/stores/room-list/RoomListStore-test.ts index 4fe59a6222..fd5562753e 100644 --- a/test/stores/room-list/RoomListStore-test.ts +++ b/test/stores/room-list/RoomListStore-test.ts @@ -29,9 +29,6 @@ import DMRoomMap from "../../../src/utils/DMRoomMap"; import { flushPromises, stubClient, upsertRoomStateEvents, mkRoom } from "../../test-utils"; import { DEFAULT_PUSH_RULES, makePushRule } from "../../test-utils/pushRules"; -// Mock out the SpaceWatcher as it messes with the prefilterConditions -jest.mock("../../../src/stores/room-list/SpaceWatcher.ts"); - describe("RoomListStore", () => { const client = stubClient(); const newRoomId = "!roomid:example.com"; @@ -94,10 +91,6 @@ describe("RoomListStore", () => { await (RoomListStore.instance as RoomListStoreClass).makeReady(client); }); - beforeEach(() => { - DMRoomMap.makeShared(client); - }); - it.each(OrderedDefaultTagIDs)("defaults to importance ordering for %s=", (tagId) => { expect(RoomListStore.instance.getTagSorting(tagId)).toBe(SortAlgorithm.Recent); }); @@ -109,11 +102,11 @@ describe("RoomListStore", () => { function createStore(): { store: RoomListStoreClass; handleRoomUpdate: jest.Mock } { const fakeDispatcher = { register: jest.fn() } as unknown as MatrixDispatcher; const store = new RoomListStoreClass(fakeDispatcher); + // @ts-ignore accessing private member to set client + store.readyStore.matrixClient = client; const handleRoomUpdate = jest.fn(); // @ts-ignore accessing private member to mock it store.algorithm.handleRoomUpdate = handleRoomUpdate; - // @ts-ignore accessing private member to set client - store.readyStore.useUnitTestClient(client); return { store, handleRoomUpdate }; } @@ -164,6 +157,7 @@ describe("RoomListStore", () => { room1.updateMyMembership(KnownMembership.Join); room2.updateMyMembership(KnownMembership.Join); room3.updateMyMembership(KnownMembership.Join); + DMRoomMap.makeShared(client); const { store } = createStore(); client.getVisibleRooms = jest.fn().mockReturnValue([room1, room2, room3]); @@ -275,6 +269,7 @@ describe("RoomListStore", () => { it("Passes the feature flag on to the client when asking for visible rooms", () => { // Given a store that we can ask for a room list + DMRoomMap.makeShared(client); const { store } = createStore(); client.getVisibleRooms = jest.fn().mockReturnValue([]); @@ -290,7 +285,7 @@ describe("RoomListStore", () => { describe("room updates", () => { const makeStore = async () => { const store = new RoomListStoreClass(defaultDispatcher); - await store.useUnitTestClient(client); + await store.start(); return store; }; diff --git a/test/stores/room-list/SlidingRoomListStore-test.ts b/test/stores/room-list/SlidingRoomListStore-test.ts index 926d1fd2f3..f667ef7dca 100644 --- a/test/stores/room-list/SlidingRoomListStore-test.ts +++ b/test/stores/room-list/SlidingRoomListStore-test.ts @@ -65,7 +65,7 @@ describe("SlidingRoomListStore", () => { describe("spaces", () => { it("alters 'filters.spaces' on the DefaultTagID.Untagged list when the selected space changes", async () => { - await store.useUnitTestClient(context.client!); // call onReady + await store.start(); // call onReady const spaceRoomId = "!foo:bar"; const p = untilEmission(store, LISTS_LOADING_EVENT, (listName, isLoading) => { @@ -92,7 +92,7 @@ describe("SlidingRoomListStore", () => { }, ); activeSpace = MetaSpace.Home; - await store.useUnitTestClient(context.client!); // call onReady + await store.start(); // call onReady expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, { filters: expect.objectContaining({ @@ -108,7 +108,7 @@ describe("SlidingRoomListStore", () => { const p = untilEmission(store, LISTS_LOADING_EVENT, (listName, isLoading) => { return listName === DefaultTagID.Untagged && !isLoading; }); - await store.useUnitTestClient(context.client!); // call onReady + await store.start(); // call onReady await p; expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith( DefaultTagID.Untagged, @@ -121,7 +121,7 @@ describe("SlidingRoomListStore", () => { }); it("includes subspaces in 'filters.spaces' when the selected space has subspaces", async () => { - await store.useUnitTestClient(context.client!); // call onReady + await store.start(); // call onReady const spaceRoomId = "!foo:bar"; const subSpace1 = "!ss1:bar"; const subSpace2 = "!ss2:bar"; @@ -168,7 +168,7 @@ describe("SlidingRoomListStore", () => { }); it("getTagsForRoom gets the tags for the room", async () => { - await store.useUnitTestClient(context.client!); + await store.start(); const roomA = "!a:localhost"; const roomB = "!b:localhost"; const keyToListData: Record }> = { @@ -200,7 +200,7 @@ describe("SlidingRoomListStore", () => { }); it("emits LISTS_UPDATE_EVENT when slidingSync lists update", async () => { - await store.useUnitTestClient(context.client!); + await store.start(); const roomA = "!a:localhost"; const roomB = "!b:localhost"; const roomC = "!c:localhost"; @@ -236,7 +236,7 @@ describe("SlidingRoomListStore", () => { }); it("sets the sticky room on the basis of the viewed room in RoomViewStore", async () => { - await store.useUnitTestClient(context.client!); + await store.start(); // seed the store with 3 rooms const roomIdA = "!a:localhost"; const roomIdB = "!b:localhost"; @@ -301,7 +301,7 @@ describe("SlidingRoomListStore", () => { }); it("gracefully handles unknown room IDs", async () => { - await store.useUnitTestClient(context.client!); + await store.start(); const roomIdA = "!a:localhost"; const roomIdB = "!b:localhost"; // does not exist const roomIdC = "!c:localhost"; diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index fe9d078f3a..ebfc6b221b 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -86,14 +86,13 @@ export function createTestClient(): MatrixClient { let txnId = 1; const client = { - startClient: jest.fn(), getHomeserverUrl: jest.fn(), getIdentityServerUrl: jest.fn(), getDomain: jest.fn().mockReturnValue("matrix.org"), getUserId: jest.fn().mockReturnValue("@userId:matrix.org"), getSafeUserId: jest.fn().mockReturnValue("@userId:matrix.org"), getUserIdLocalpart: jest.fn().mockResolvedValue("userId"), - getUser: jest.fn().mockReturnValue({ on: jest.fn(), off: jest.fn(), removeListener: jest.fn() }), + getUser: jest.fn().mockReturnValue({ on: jest.fn(), off: jest.fn() }), getDevice: jest.fn(), getDeviceId: jest.fn().mockReturnValue("ABCDEFGHI"), getStoredCrossSigningForUser: jest.fn(), @@ -134,8 +133,6 @@ export function createTestClient(): MatrixClient { getVerificationRequestsToDeviceInProgress: jest.fn().mockReturnValue([]), setDeviceIsolationMode: jest.fn(), }), - initCrypto: jest.fn(), - initRustCrypto: jest.fn(), getPushActionsForEvent: jest.fn(), getRoom: jest.fn().mockImplementation((roomId) => mkStubRoom(roomId, "My room", client)), @@ -183,7 +180,6 @@ export function createTestClient(): MatrixClient { getSyncState: jest.fn().mockReturnValue("SYNCING"), generateClientSecret: () => "t35tcl1Ent5ECr3T", isGuest: jest.fn().mockReturnValue(false), - setGuest: jest.fn(), getRoomHierarchy: jest.fn().mockReturnValue({ rooms: [], }), @@ -281,7 +277,6 @@ export function createTestClient(): MatrixClient { isFallbackICEServerAllowed: jest.fn().mockReturnValue(false), getAuthIssuer: jest.fn(), getOrCreateFilter: jest.fn(), - setNotifTimelineSet: jest.fn(), } as unknown as MatrixClient; client.reEmitter = new ReEmitter(client); From c71dc6b0f8b9353a02b57fe098e44cf8a206f596 Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 10 Oct 2024 15:52:45 +0100 Subject: [PATCH 6/7] Upgrade emojibase-bindings and remove local handling of emoticon variations (#127) * Updgrade emojibase-bindings and remove local handling of emoticon variations * bump bindings to fix issue of mission emojis with version == 0 * update lockfile * test emoji variation in RTE --- package.json | 2 +- .../views/rooms/BasicMessageComposer.tsx | 6 ++-- .../wysiwyg_composer/hooks/useSuggestion.ts | 4 ++- .../views/rooms/BasicMessageComposer-test.tsx | 1 + .../components/WysiwygComposer-test.tsx | 12 ++++++++ yarn.lock | 28 +++++++++---------- 6 files changed, 34 insertions(+), 19 deletions(-) diff --git a/package.json b/package.json index 49d0f1ffb3..ea3ab91b79 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,7 @@ "dependencies": { "@babel/runtime": "^7.12.5", "@matrix-org/analytics-events": "^0.26.0", - "@matrix-org/emojibase-bindings": "^1.1.2", + "@matrix-org/emojibase-bindings": "^1.3.3", "@vector-im/matrix-wysiwyg": "2.37.13", "@matrix-org/react-sdk-module-api": "^2.4.0", "@matrix-org/spec": "^1.7.0", diff --git a/src/components/views/rooms/BasicMessageComposer.tsx b/src/components/views/rooms/BasicMessageComposer.tsx index 012e99669a..0add0c1027 100644 --- a/src/components/views/rooms/BasicMessageComposer.tsx +++ b/src/components/views/rooms/BasicMessageComposer.tsx @@ -201,9 +201,9 @@ export default class BasicMessageEditor extends React.Component // so xd will not match if the string was "mixd 123456" // and we are lookinh at xd 123456 part of the string if (emoticonMatch && (n >= 0 || emoticonMatch.index !== 0)) { - const query = emoticonMatch[1].replace("-", ""); - // try both exact match and lower-case, this means that xd won't match xD but :P will match :p - const data = EMOTICON_TO_EMOJI.get(query) || EMOTICON_TO_EMOJI.get(query.toLowerCase()); + const query = emoticonMatch[1]; + // variations of plaintext emoitcons(E.g. :P vs :p vs :-P) are handled upstream by the emojibase-bindings library + const data = EMOTICON_TO_EMOJI.get(query); if (data) { const { partCreator } = model; diff --git a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts index 7fdd630f10..5b4957a9c5 100644 --- a/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts +++ b/src/components/views/rooms/wysiwyg_composer/hooks/useSuggestion.ts @@ -388,7 +388,9 @@ function shouldIncrementEndIndex(text: string, index: number): boolean { */ export function getMappedSuggestion(text: string, isAutoReplaceEmojiEnabled?: boolean): MappedSuggestion | null { if (isAutoReplaceEmojiEnabled) { - const emoji = EMOTICON_TO_EMOJI.get(text.toLocaleLowerCase()); + // variations of plaintext emoitcons(E.g. :P vs :p vs :-P) are handled upstream by the emojibase-bindings/emojibase libraries. + // See rules for variations here https://github.com/milesj/emojibase/blob/master/packages/core/src/generateEmoticonPermutations.ts#L3-L32 + const emoji = EMOTICON_TO_EMOJI.get(text); if (emoji?.unicode) { return { keyChar: "", text: emoji.unicode, type: "custom" }; } diff --git a/test/components/views/rooms/BasicMessageComposer-test.tsx b/test/components/views/rooms/BasicMessageComposer-test.tsx index f498479a68..a46dba0f1e 100644 --- a/test/components/views/rooms/BasicMessageComposer-test.tsx +++ b/test/components/views/rooms/BasicMessageComposer-test.tsx @@ -63,6 +63,7 @@ describe("BasicMessageComposer", () => { { before: ":-D", after: "😄" }, { before: ":D", after: "😄" }, { before: ":3", after: "😽" }, + { before: "=-]", after: "🙂" }, ]; const input = screen.getByRole("textbox"); diff --git a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx index e58b21db6e..f32a4e7a4e 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/WysiwygComposer-test.tsx @@ -435,6 +435,18 @@ describe("WysiwygComposer", () => { inputType: "insertText", }); + await waitFor(() => expect(onChange).toHaveBeenNthCalledWith(3, expect.stringContaining("😛"))); + }); + it("typing a space to trigger an emoji varitation replacement", async () => { + fireEvent.input(screen.getByRole("textbox"), { + data: ":-P", + inputType: "insertText", + }); + fireEvent.input(screen.getByRole("textbox"), { + data: " ", + inputType: "insertText", + }); + await waitFor(() => expect(onChange).toHaveBeenNthCalledWith(3, expect.stringContaining("😛"))); }); }); diff --git a/yarn.lock b/yarn.lock index 5c995a2538..c34ac3a9f5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1898,13 +1898,13 @@ resolved "https://registry.yarnpkg.com/@matrix-org/analytics-events/-/analytics-events-0.26.0.tgz#7c8f8f924d8313c87951a0e941640ef8ff78f3d6" integrity sha512-cjKZBejajUG8wPhVygMkBTwTLdEn74luUP6g6RjCUqPR3RYIl3NVi58Zil8CWfRTILb4wVLCPpAvehgXJn1HnQ== -"@matrix-org/emojibase-bindings@^1.1.2": - version "1.1.3" - resolved "https://registry.yarnpkg.com/@matrix-org/emojibase-bindings/-/emojibase-bindings-1.1.3.tgz#d35f0367d586d83261564662b6bb67fda8845d68" - integrity sha512-ljr0kPerx8yUc4JVJz7japebb1ZbtGH4V4cvlO2LYgTg+warjagDwkJ5x+ZUVuTU6MH8x0LrUxmkqVgmSoQyWA== +"@matrix-org/emojibase-bindings@^1.3.3": + version "1.3.3" + resolved "https://registry.yarnpkg.com/@matrix-org/emojibase-bindings/-/emojibase-bindings-1.3.3.tgz#cee82a739c0866bf3100b03755647ace1f3ba6ef" + integrity sha512-GwuZdmF+wZT34RKehQYjTzdgba1ju2W3FM4jPJfwqh0jUxVXZLb+6b6dV3lna6/7EDzgGvOMwTwCAolILDwS0g== dependencies: - emojibase "^15.0.0" - emojibase-data "^15.0.0" + emojibase "^15.3.1" + emojibase-data "^15.3.1" "@matrix-org/matrix-sdk-crypto-wasm@^7.0.0": version "7.0.0" @@ -4431,20 +4431,20 @@ emoji-regex@^9.2.2: resolved "https://registry.yarnpkg.com/emoji-regex/-/emoji-regex-9.2.2.tgz#840c8803b0d8047f4ff0cf963176b32d4ef3ed72" integrity sha512-L18DaJsXSUk2+42pv8mLs5jJT2hqFkFE4j21wOmgbUqsZ2hL72NsUU785g9RXgo3s0ZNgVl42TiHp3ZtOv/Vyg== -emojibase-data@^15.0.0: - version "15.2.0" - resolved "https://registry.yarnpkg.com/emojibase-data/-/emojibase-data-15.2.0.tgz#475a786c091a101ef4bcf57227771c6260ee39b2" - integrity sha512-hDiw4ugxnI4pcVQO+73NlKx6aZP/A+BAPfDgK/3A83RVbHZa0Ut6GHpd5r5XUV9G7BZhKejlIRuxhXialpbt6Q== +emojibase-data@^15.3.1: + version "15.3.2" + resolved "https://registry.yarnpkg.com/emojibase-data/-/emojibase-data-15.3.2.tgz#2742246bfe14f16a7829b42ca156dec09934cf85" + integrity sha512-TpDyTDDTdqWIJixV5sTA6OQ0P0JfIIeK2tFRR3q56G9LK65ylAZ7z3KyBXokpvTTJ+mLUXQXbLNyVkjvnTLE+A== emojibase-regex@15.3.2: version "15.3.2" resolved "https://registry.yarnpkg.com/emojibase-regex/-/emojibase-regex-15.3.2.tgz#5175231715b86d4b437754527288844a6c29318f" integrity sha512-ue6BVeb2qu33l97MkxcOoyMJlg6Tug3eTv2z1at+M9TjvlWKvdmAPvZIDG1JbT2RH3FSyJNLucO5K5H/yxT03w== -emojibase@^15.0.0: - version "15.2.0" - resolved "https://registry.yarnpkg.com/emojibase/-/emojibase-15.2.0.tgz#0a175d4218f4a1600f2ada6b3a3184cef1b3ec7b" - integrity sha512-gB6rIVtyJPersQvAo4nOGYPeILMhlcfZdiwCWVeRAtkJ7sm0tExZETGyLhrTQcHvZQhDEYI1vlCeqUhn5gZkQA== +emojibase@^15.3.1: + version "15.3.1" + resolved "https://registry.yarnpkg.com/emojibase/-/emojibase-15.3.1.tgz#7f6ff5482486f23e59a457de64e974bd35f3c9a3" + integrity sha512-GNsjHnG2J3Ktg684Fs/vZR/6XpOSkZPMAv85EHrr6br2RN2cJNwdS4am/3YSK3y+/gOv2kmoK3GGdahXdMxg2g== emojis-list@^3.0.0: version "3.0.0" From 771d4a84171a71c02dad2f5c69526de29cbf6a5d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 11 Oct 2024 12:48:46 +0100 Subject: [PATCH 7/7] Fix flaky crypto playwright tests (#143) * Playwright: wait for sync to arrive after joining rooms Fix a couple of flaky tests which were not waiting for the /sync to complete after joining a room. * Playwright: add a comment about a broken helper * playwright: fix more flakiness in the shields test This bit can take a while as well. * Update playwright/pages/client.ts Co-authored-by: R Midhun Suresh * Add a timeout to `awaitRoomMembership` --------- Co-authored-by: R Midhun Suresh --- playwright/e2e/crypto/crypto.spec.ts | 7 ++- playwright/e2e/crypto/event-shields.spec.ts | 9 ++-- playwright/e2e/utils.ts | 8 ++++ playwright/pages/client.ts | 48 +++++++++++++++++++++ 4 files changed, 68 insertions(+), 4 deletions(-) diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index f3a4820ebe..e520d971ea 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -43,6 +43,7 @@ const testMessages = async (page: Page, bob: Bot, bobRoomId: string) => { }; const bobJoin = async (page: Page, bob: Bot) => { + // Wait for Bob to get the invite await bob.evaluate(async (cli) => { const bobRooms = cli.getRooms(); if (!bobRooms.length) { @@ -55,9 +56,13 @@ const bobJoin = async (page: Page, bob: Bot) => { }); } }); - const roomId = await bob.joinRoomByName("Alice"); + const roomId = await bob.joinRoomByName("Alice"); await expect(page.getByText("Bob joined the room")).toBeVisible(); + + // Even though Alice has seen Bob's join event, Bob may not have done so yet. Wait for the sync to arrive. + await bob.awaitRoomMembership(roomId); + return roomId; }; diff --git a/playwright/e2e/crypto/event-shields.spec.ts b/playwright/e2e/crypto/event-shields.spec.ts index fa9d1959da..0544a7c904 100644 --- a/playwright/e2e/crypto/event-shields.spec.ts +++ b/playwright/e2e/crypto/event-shields.spec.ts @@ -33,7 +33,7 @@ test.describe("Cryptography", function () { await app.client.bootstrapCrossSigning(aliceCredentials); await autoJoin(bob); - // create an encrypted room + // create an encrypted room, and wait for Bob to join it. testRoomId = await createSharedRoomWithUser(app, bob.credentials.userId, { name: "TestRoom", initial_state: [ @@ -46,6 +46,9 @@ test.describe("Cryptography", function () { }, ], }); + + // Even though Alice has seen Bob's join event, Bob may not have done so yet. Wait for the sync to arrive. + await bob.awaitRoomMembership(testRoomId); }); test("should show the correct shield on e2e events", async ({ @@ -287,9 +290,9 @@ test.describe("Cryptography", function () { // Let our app start syncing again await app.client.network.goOnline(); - // Wait for the messages to arrive + // Wait for the messages to arrive. It can take quite a while for the sync to wake up. const last = page.locator(".mx_EventTile_last"); - await expect(last).toContainText("test encrypted from unverified"); + await expect(last).toContainText("test encrypted from unverified", { timeout: 20000 }); const lastE2eIcon = last.locator(".mx_EventTile_e2eIcon"); await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/); await lastE2eIcon.focus(); diff --git a/playwright/e2e/utils.ts b/playwright/e2e/utils.ts index b357b5ca99..a2bcc0f29a 100644 --- a/playwright/e2e/utils.ts +++ b/playwright/e2e/utils.ts @@ -20,6 +20,14 @@ import { Client } from "../pages/client"; * @param client Client instance that can be user or bot * @param roomId room id to find room and check * @param predicate defines condition that is used to check the room state + * + * FIXME this does not do what it is supposed to do, and I think it is unfixable. + * `page.exposeFunction` adds a function which returns a Promise. `window[predicateId](room)` therefore + * always returns a truthy value (a Promise). But even if you fix that: as far as I can tell, the Room is + * just passed to the callback function as a JSON blob: you cannot actually call any methods on it, so the + * callback is useless. + * + * @deprecated This function is broken. */ export async function waitForRoom( page: Page, diff --git a/playwright/pages/client.ts b/playwright/pages/client.ts index 06e05fdcfa..7d62f42ae0 100644 --- a/playwright/pages/client.ts +++ b/playwright/pages/client.ts @@ -289,6 +289,54 @@ export class Client { await client.evaluate((client, { roomId, userId }) => client.unban(roomId, userId), { roomId, userId }); } + /** + * Wait for the client to have specific membership of a given room + * + * This is often useful after joining a room, when we need to wait for the sync loop to catch up. + * + * Times out with an error after 1 second. + * + * @param roomId - ID of the room to check + * @param membership - required membership. + */ + public async awaitRoomMembership(roomId: string, membership: string = "join") { + await this.evaluate( + (cli: MatrixClient, { roomId, membership }) => { + const isReady = () => { + // Fetch the room on each check, because we get a different instance before and after the join arrives. + const room = cli.getRoom(roomId); + const myMembership = room?.getMyMembership(); + // @ts-ignore access to private field "logger" + cli.logger.info(`waiting for room ${roomId}: membership now ${myMembership}`); + return myMembership === membership; + }; + if (isReady()) return; + + const timeoutPromise = new Promise((resolve) => setTimeout(resolve, 1000)).then(() => { + const room = cli.getRoom(roomId); + const myMembership = room?.getMyMembership(); + throw new Error( + `Timeout waiting for room ${roomId} membership (now '${myMembership}', wanted '${membership}')`, + ); + }); + + const readyPromise = new Promise((resolve) => { + async function onEvent() { + if (isReady()) { + cli.removeListener(window.matrixcs.ClientEvent.Event, onEvent); + resolve(); + } + } + + cli.on(window.matrixcs.ClientEvent.Event, onEvent); + }); + + return Promise.race([timeoutPromise, readyPromise]); + }, + { roomId, membership }, + ); + } + /** * @param {MatrixEvent} event * @param {ReceiptType} receiptType