From 939eac5a669152bc562ee5e1418847dc6fb0e18f Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Tue, 4 Mar 2025 19:37:51 +0100 Subject: [PATCH 1/8] change lk log level to warn (#3049) --- src/main.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.tsx b/src/main.tsx index 2459997f..fb29cf66 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -27,7 +27,7 @@ import { Initializer } from "./initializer"; initRageshake().catch((e) => { logger.error("Failed to initialize rageshake", e); }); -setLKLogLevel("debug"); +setLKLogLevel("warn"); setLKLogExtension((level, msg, context) => { // we pass a synthetic logger name of "livekit" to the rageshake to make it easier to read global.mx_rage_logger.log(level, "livekit", msg, context); From 771397389c6490e21508c0c2afbe2a3fdeac8e53 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 5 Mar 2025 05:44:10 -0500 Subject: [PATCH 2/8] Prevent PRs with the X-Blocked label from being merged (#3041) Copied from matrix-js-sdk (https://github.com/matrix-org/matrix-js-sdk/blob/develop/.github/workflows/pull_request.yaml#L31) --- .github/workflows/blocked.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .github/workflows/blocked.yaml diff --git a/.github/workflows/blocked.yaml b/.github/workflows/blocked.yaml new file mode 100644 index 00000000..d6e592cb --- /dev/null +++ b/.github/workflows/blocked.yaml @@ -0,0 +1,17 @@ +name: Prevent blocked +on: + pull_request: + types: [opened, labeled, unlabeled] +jobs: + prevent-blocked: + name: Prevent blocked + runs-on: ubuntu-latest + permissions: + pull-requests: read + steps: + - name: Add notice + uses: actions/github-script@v7 + if: contains(github.event.pull_request.labels.*.name, 'X-Blocked') + with: + script: | + core.setFailed("PR has been labeled with X-Blocked; it cannot be merged."); From b5f5edba0925b70c0466c5417bd77f6499ce2179 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 4 Mar 2025 15:06:47 -0500 Subject: [PATCH 3/8] Fix the control flow of GroupCallView render function 2bb5b020e60c3d8b6034be799db42fa3d3164cc4 refactored the end of the GroupCallView render function to not use any early returns, and clumsily failed to account for the fall-through case that makes returnToLobby work (as opposed to sitting on a blank screen). --- src/room/GroupCallView.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 95d1d12c..66f14821 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -495,9 +495,7 @@ export const GroupCallView: FC = ({ } } else if (left && widget !== null) { // Left in widget mode: - if (!returnToLobby) { - body = null; - } + body = returnToLobby ? lobbyView : null; } else if (preload || skipLobby) { body = null; } else { From 28c45c61072126023381446877fb99c4c36fd8ac Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 4 Mar 2025 15:09:59 -0500 Subject: [PATCH 4/8] Avoid closing the widget in returnToLobby mode If returnToLobby is enabled then we obviously want to keep the widget open once the user leaves the call. --- src/UrlParams.test.ts | 4 +-- src/UrlParams.ts | 2 +- src/rtcSessionHelpers.test.ts | 57 +++++++++++++++++++++++------------ src/rtcSessionHelpers.ts | 3 +- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/UrlParams.test.ts b/src/UrlParams.test.ts index 8e185abc..dce46754 100644 --- a/src/UrlParams.test.ts +++ b/src/UrlParams.test.ts @@ -110,8 +110,8 @@ describe("UrlParams", () => { }); describe("returnToLobby", () => { - it("is true in SPA mode", () => { - expect(getUrlParams("?returnToLobby=false").returnToLobby).toBe(true); + it("is false in SPA mode", () => { + expect(getUrlParams("?returnToLobby=true").returnToLobby).toBe(false); }); it("defaults to false in widget mode", () => { diff --git a/src/UrlParams.ts b/src/UrlParams.ts index 61b777c7..23b7ecbd 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -264,7 +264,7 @@ export const getUrlParams = ( "skipLobby", isWidget && intent === UserIntent.StartNewCall, ), - returnToLobby: isWidget ? parser.getFlagParam("returnToLobby") : true, + returnToLobby: isWidget ? parser.getFlagParam("returnToLobby") : false, theme: parser.getParam("theme"), viaServers: !isWidget ? parser.getParam("viaServers") : null, homeserver: !isWidget ? parser.getParam("homeserver") : null, diff --git a/src/rtcSessionHelpers.test.ts b/src/rtcSessionHelpers.test.ts index 21ee2cd3..972d6e75 100644 --- a/src/rtcSessionHelpers.test.ts +++ b/src/rtcSessionHelpers.test.ts @@ -6,7 +6,7 @@ Please see LICENSE in the repository root for full details. */ import { type MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; -import { expect, test, vi } from "vitest"; +import { expect, onTestFinished, test, vi } from "vitest"; import { AutoDiscovery } from "matrix-js-sdk/src/autodiscovery"; import EventEmitter from "events"; @@ -15,11 +15,17 @@ import { mockConfig } from "./utils/test"; import { ElementWidgetActions, widget } from "./widget"; import { ErrorCode } from "./utils/errors.ts"; +const getUrlParams = vi.hoisted(() => vi.fn(() => ({}))); +vi.mock("./UrlParams", () => ({ getUrlParams })); + const actualWidget = await vi.hoisted(async () => vi.importActual("./widget")); vi.mock("./widget", () => ({ ...actualWidget, widget: { - api: { transport: { send: vi.fn(), reply: vi.fn(), stop: vi.fn() } }, + api: { + setAlwaysOnScreen: (): void => {}, + transport: { send: vi.fn(), reply: vi.fn(), stop: vi.fn() }, + }, lazyActions: new EventEmitter(), }, })); @@ -109,34 +115,45 @@ test("It joins the correct Session", async () => { ); }); -test("leaveRTCSession closes the widget on a normal hangup", async () => { +async function testLeaveRTCSession( + cause: "user" | "error", + expectClose: boolean, +): Promise { vi.clearAllMocks(); const session = { leaveRoomSession: vi.fn() } as unknown as MatrixRTCSession; - await leaveRTCSession(session, "user"); + await leaveRTCSession(session, cause); expect(session.leaveRoomSession).toHaveBeenCalled(); expect(widget!.api.transport.send).toHaveBeenCalledWith( ElementWidgetActions.HangupCall, expect.anything(), ); - expect(widget!.api.transport.send).toHaveBeenCalledWith( - ElementWidgetActions.Close, - expect.anything(), - ); + if (expectClose) { + expect(widget!.api.transport.send).toHaveBeenCalledWith( + ElementWidgetActions.Close, + expect.anything(), + ); + expect(widget!.api.transport.stop).toHaveBeenCalled(); + } else { + expect(widget!.api.transport.send).not.toHaveBeenCalledWith( + ElementWidgetActions.Close, + expect.anything(), + ); + expect(widget!.api.transport.stop).not.toHaveBeenCalled(); + } +} + +test("leaveRTCSession closes the widget on a normal hangup", async () => { + await testLeaveRTCSession("user", true); }); test("leaveRTCSession doesn't close the widget on a fatal error", async () => { - vi.clearAllMocks(); - const session = { leaveRoomSession: vi.fn() } as unknown as MatrixRTCSession; - await leaveRTCSession(session, "error"); - expect(session.leaveRoomSession).toHaveBeenCalled(); - expect(widget!.api.transport.send).toHaveBeenCalledWith( - ElementWidgetActions.HangupCall, - expect.anything(), - ); - expect(widget!.api.transport.send).not.toHaveBeenCalledWith( - ElementWidgetActions.Close, - expect.anything(), - ); + await testLeaveRTCSession("error", false); +}); + +test("leaveRTCSession doesn't close the widget when returning to lobby", async () => { + getUrlParams.mockReturnValue({ returnToLobby: true }); + onTestFinished(() => void getUrlParams.mockReset()); + await testLeaveRTCSession("user", false); }); test("It fails with configuration error if no live kit url config is set in fallback", async () => { diff --git a/src/rtcSessionHelpers.ts b/src/rtcSessionHelpers.ts index 719af998..943eaf82 100644 --- a/src/rtcSessionHelpers.ts +++ b/src/rtcSessionHelpers.ts @@ -19,6 +19,7 @@ import { PosthogAnalytics } from "./analytics/PosthogAnalytics"; import { Config } from "./config/Config"; import { ElementWidgetActions, widget, type WidgetHelpers } from "./widget"; import { MatrixRTCFocusMissingError } from "./utils/errors.ts"; +import { getUrlParams } from "./UrlParams.ts"; const FOCI_WK_KEY = "org.matrix.msc4143.rtc_foci"; @@ -149,7 +150,7 @@ const widgetPostHangupProcedure = async ( } // On a normal user hangup we can shut down and close the widget. But if an // error occurs we should keep the widget open until the user reads it. - if (cause === "user") { + if (cause === "user" && !getUrlParams().returnToLobby) { try { await widget.api.transport.send(ElementWidgetActions.Close, {}); } catch (e) { From ec73e7fa860da7a926c9ff767f5444861417f2c6 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 5 Mar 2025 09:18:31 -0500 Subject: [PATCH 5/8] Use the configured brand name in OpenGraph tags Overlooked some of these when reviewing https://github.com/element-hq/element-call/pull/3006. --- public/index.html | 10 +++++----- vite.config.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/public/index.html b/public/index.html index bf26d8ec..579f5a00 100644 --- a/public/index.html +++ b/public/index.html @@ -8,26 +8,26 @@ name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0" /> - <%- title %> + <%- brand %> - + - + diff --git a/vite.config.js b/vite.config.js index 4c9871a2..8f067357 100644 --- a/vite.config.js +++ b/vite.config.js @@ -29,7 +29,7 @@ export default defineConfig(({ mode }) => { }), htmlTemplate.default({ data: { - title: env.VITE_PRODUCT_NAME || "Element Call", + brand: env.VITE_PRODUCT_NAME || "Element Call", }, }), From 5b1ea4501d379324427401af94a0cc0f8ecb183e Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 5 Mar 2025 09:25:52 -0500 Subject: [PATCH 6/8] Send a 'join' action when entering the call Following a75952cf77cea0fc67058defb43850d7ff153163, this is one more upgrade to the widget communication that I'd like to make within this release cycle. The motivating issue is https://github.com/element-hq/element-web/issues/29429. Fundamentally, without a 'join' action, the only info Element Web can use to determine whether it's joined the call is whether a MatrixRTC membership exists. But membership state events can inaccurately represent the client's actual state (whether because delayed events aren't supported, or because the delayed event hasn't timed out yet), so I suggest we send a 'join' action here just as we do in the Element Web Jitsi wrapper (https://github.com/element-hq/element-web/blob/e9a3625bd6e9a64f216e3caeabca66f48b649332/src/vector/jitsi/index.ts#L503) to let Element Web tap directly into the widget's local state. (This will need additional Element Web changes, but is certainly backwards compatible.) --- src/rtcSessionHelpers.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/rtcSessionHelpers.ts b/src/rtcSessionHelpers.ts index 719af998..9fb4d3af 100644 --- a/src/rtcSessionHelpers.ts +++ b/src/rtcSessionHelpers.ts @@ -124,6 +124,13 @@ export async function enterRTCSession( makeKeyDelay: matrixRtcSessionConfig?.key_rotation_on_leave_delay, }, ); + if (widget) { + try { + await widget.api.transport.send(ElementWidgetActions.JoinCall, {}); + } catch (e) { + logger.error("Failed to send join action", e); + } + } } const widgetPostHangupProcedure = async ( From 359812d8b1b9ab20fdc69f2220583d2f6c0b7d25 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 5 Mar 2025 10:40:37 -0500 Subject: [PATCH 7/8] Explain why returnToLobby is false in SPA --- src/UrlParams.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/UrlParams.ts b/src/UrlParams.ts index 23b7ecbd..fda4a95f 100644 --- a/src/UrlParams.ts +++ b/src/UrlParams.ts @@ -264,6 +264,8 @@ export const getUrlParams = ( "skipLobby", isWidget && intent === UserIntent.StartNewCall, ), + // In SPA mode the user should always exit to the home screen when hanging + // up, rather than being sent back to the lobby returnToLobby: isWidget ? parser.getFlagParam("returnToLobby") : false, theme: parser.getParam("theme"), viaServers: !isWidget ? parser.getParam("viaServers") : null, From c9f2a1c94370cb23500e307759a51a4cd9312105 Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Fri, 7 Mar 2025 11:18:28 +0100 Subject: [PATCH 8/8] Reduce redundant calculations of display name map (#3062) * Use share() on fromEvent() so that we multiplex subscribers onto the event emitter * . * . * Comment * Comment --------- Co-authored-by: Hugh Nimmo-Smith --- src/state/CallViewModel.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index f634233e..ce104396 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -496,6 +496,10 @@ export class CallViewModel extends ViewModel { } return displaynameMap; }), + // It turns out that doing the disambiguation above is rather expensive on Safari (10x slower + // than on Chrome/Firefox). This means it is important that we share() the result so that we + // don't do this work more times than we need to. This is achieve through the state() operator: + this.scope.state(), ); /**