mirrors/element-web
1
0
Fork 0
mirror of https://github.com/vector-im/element-web.git synced 2025-12-26 19:51:40 +01:00
Code Issues Projects Releases Wiki Activity

Fix widget initialization in React development mode (#30463)

Browse Source 
Since the upgrade to React 19, widget initialization (most notably affecting group calls) has been broken in development mode. This is because React now executes all callback refs twice, and the callback ref that receives the widget's iframe was not prepared to deal with that. I've fixed this by creating and attaching the iframe to the DOM in the callback ref, which allows us to properly couple its lifetime to that of the StopGapWidget. I've also added some insurance against strict mode-style races in StopGapWidget (doesn't hurt).
This commit is contained in:
Robin 2025-08-06 14:17:00 +02:00 committed by GitHub
parent 8a550cf3f6
commit 15f1291cbc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 78 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

120
src/components/views/elements/AppTile.tsx
View File

@ -62,6 +62,20 @@ import { parseUrl } from "../../../utils/UrlUtils";
import RightPanelStore from "../../../stores/right-panel/RightPanelStore.ts";
import { RightPanelPhases } from "../../../stores/right-panel/RightPanelStorePhases.ts";
// Note that there is advice saying allow-scripts shouldn't be used with allow-same-origin
// because that would allow the iframe to programmatically remove the sandbox attribute, but
// this would only be for content hosted on the same origin as the element client: anything
// hosted on the same origin as the client will get the same access as if you clicked
// a link to it.
const sandboxFlags =
"allow-forms allow-popups allow-popups-to-escape-sandbox " +
"allow-same-origin allow-scripts allow-presentation allow-downloads";
// Additional iframe feature permissions
// (see - https://sites.google.com/a/chromium.org/dev/Home/chromium-security/deprecating-permissions-in-cross-origin-iframes and https://wicg.github.io/feature-policy/)
const iframeFeatures =
"microphone; camera; encrypted-media; autoplay; display-capture; clipboard-write; clipboard-read;";
interface IProps {
app: IWidget | IApp;
// If room is not specified then it is an account level widget
@ -138,7 +152,7 @@ export default class AppTile extends React.Component<IProps, IState> {
};
private contextMenuButton = createRef<any>();
private iframe?: HTMLIFrameElement; // ref to the iframe (callback style)
private iframeParent: HTMLElement | null = null; // parent div of the iframe
private allowedWidgetsWatchRef?: string;
private persistKey: string;
private sgWidget?: StopGapWidget;
@ -397,18 +411,46 @@ export default class AppTile extends React.Component<IProps, IState> {
});
}
/**
* Creates the widget iframe and opens communication with the widget.
*/
private startMessaging(): void {
try {
this.sgWidget?.startMessaging(this.iframe!);
} catch (e) {
logger.error("Failed to start widget", e);
}
// We create the iframe ourselves rather than leaving the job to React,
// because we need the lifetime of the messaging and the iframe to be
// the same; we don't want strict mode, for instance, to cause the
// messaging to restart (lose its state) without also killing the widget
const iframe = document.createElement("iframe");
iframe.title = WidgetUtils.getWidgetName(this.props.app);
iframe.allow = iframeFeatures;
iframe.src = this.sgWidget!.embedUrl;
iframe.allowFullscreen = true;
iframe.sandbox = sandboxFlags;
this.iframeParent!.appendChild(iframe);
// In order to start the widget messaging we need iframe.contentWindow
// to exist. Waiting until the next layout gives the browser a chance to
// initialize it.
requestAnimationFrame(() => {
// Handle the race condition (seen in strict mode) where the element
// is added and then removed before we enter this callback
if (iframe.parentElement === null) return;
try {
this.sgWidget?.startMessaging(iframe);
} catch (e) {
logger.error("Failed to start widget", e);
}
});
}
private iframeRefChange = (ref: HTMLIFrameElement): void => {
this.iframe = ref;
/**
* Callback ref for the parent div of the iframe.
*/
private iframeParentRef = (element: HTMLElement | null): void => {
// Detach the existing iframe (if any) from the document so we know not
// to do anything further with it, like starting up the messaging
this.iframeParent?.querySelector("iframe")?.remove();
this.iframeParent = element;
if (this.unmounted) return;
if (ref) {
if (element && this.sgWidget) {
this.startMessaging();
} else {
this.resetWidget(this.props);
@ -426,24 +468,8 @@ export default class AppTile extends React.Component<IProps, IState> {
/**
* Ends all widget interaction, such as cancelling calls and disabling webcams.
* @private
* @returns {Promise<*>} Resolves when the widget is terminated, or timeout passed.
*/
private async endWidgetActions(): Promise<void> {
// widget migration dev note: async to maintain signature
// HACK: This is a really dirty way to ensure that Jitsi cleans up
// its hold on the webcam. Without this, the widget holds a media
// stream open, even after death. See https://github.com/vector-im/element-web/issues/7351
if (this.iframe) {
// In practice we could just do `+= ''` to trick the browser
// into thinking the URL changed, however I can foresee this
// being optimized out by a browser. Instead, we'll just point
// the iframe at a page that is reasonably safe to use in the
// event the iframe doesn't wink away.
// This is relative to where the Element instance is located.
this.iframe.src = "about:blank";
}
private endWidgetActions(): void {
if (WidgetType.JITSI.matches(this.props.app.type) && this.props.room) {
LegacyCallHandler.instance.hangupCallApp(this.props.room.roomId);
}
@ -457,6 +483,7 @@ export default class AppTile extends React.Component<IProps, IState> {
this.sgWidget?.stopMessaging({ forceDestroy: true });
}
private onWidgetReady = (): void => {
this.setState({ loading: false });
};
@ -554,16 +581,11 @@ export default class AppTile extends React.Component<IProps, IState> {
}
private reload(): void {
this.endWidgetActions().then(() => {
// reset messaging
this.resetWidget(this.props);
this.startMessaging();
if (this.iframe && this.sgWidget) {
// Reload iframe
this.iframe.src = this.sgWidget.embedUrl;
}
});
this.endWidgetActions();
// reset messaging
this.resetWidget(this.props);
this.iframeParent?.querySelector("iframe")?.remove();
this.startMessaging();
}
// TODO replace with full screen interactions
@ -621,20 +643,6 @@ export default class AppTile extends React.Component<IProps, IState> {
public render(): React.ReactNode {
let appTileBody: JSX.Element | undefined;
// Note that there is advice saying allow-scripts shouldn't be used with allow-same-origin
// because that would allow the iframe to programmatically remove the sandbox attribute, but
// this would only be for content hosted on the same origin as the element client: anything
// hosted on the same origin as the client will get the same access as if you clicked
// a link to it.
const sandboxFlags =
"allow-forms allow-popups allow-popups-to-escape-sandbox " +
"allow-same-origin allow-scripts allow-presentation allow-downloads";
// Additional iframe feature permissions
// (see - https://sites.google.com/a/chromium.org/dev/Home/chromium-security/deprecating-permissions-in-cross-origin-iframes and https://wicg.github.io/feature-policy/)
const iframeFeatures =
"microphone; camera; encrypted-media; autoplay; display-capture; clipboard-write; " + "clipboard-read;";
const appTileBodyClass = classNames({
"mx_AppTileBody": true,
"mx_AppTileBody--large": !this.props.miniMode,
@ -654,8 +662,6 @@ export default class AppTile extends React.Component<IProps, IState> {
</div>
);
const widgetTitle = WidgetUtils.getWidgetName(this.props.app);
if (this.sgWidget === null) {
appTileBody = (
<div className={appTileBodyClass} style={appTileBodyStyles}>
@ -692,16 +698,8 @@ export default class AppTile extends React.Component<IProps, IState> {
} else if (this.sgWidget) {
appTileBody = (
<>
<div className={appTileBodyClass} style={appTileBodyStyles}>
<div className={appTileBodyClass} style={appTileBodyStyles} ref={this.iframeParentRef}>
{this.state.loading && loadingElement}
<iframe
title={widgetTitle}
allow={iframeFeatures}
ref={this.iframeRefChange}
src={this.sgWidget.embedUrl}
allowFullScreen={true}
sandbox={sandboxFlags}
/>
</div>
{this.props.overlay}
</>

30
src/stores/widgets/StopGapWidget.ts
View File

@ -154,6 +154,7 @@ export class ElementWidget extends Widget {
export class StopGapWidget extends EventEmitter {
private client: MatrixClient;
private iframe: HTMLIFrameElement | null = null;
private messaging: ClientWidgetApi | null = null;
private mockWidget: ElementWidget;
private scalarToken?: string;
@ -242,10 +243,6 @@ export class StopGapWidget extends EventEmitter {
return parsed.toString().replace(/%24/g, "$");
}
public get started(): boolean {
return !!this.messaging;
}
private onThemeChange = (theme: string): void => {
this.messaging?.updateTheme({ name: theme });
};
@ -278,9 +275,10 @@ export class StopGapWidget extends EventEmitter {
* This starts the messaging for the widget if it is not in the state `started` yet.
* @param iframe the iframe the widget should use
*/
public startMessaging(iframe: HTMLIFrameElement): any {
if (this.started) return;
public startMessaging(iframe: HTMLIFrameElement): void {
if (this.messaging !== null) return;
this.iframe = iframe;
const allowedCapabilities = this.appTileProps.whitelistCapabilities || [];
const driver = new StopGapWidgetDriver(
allowedCapabilities,
@ -478,16 +476,26 @@ export class StopGapWidget extends EventEmitter {
* @param opts
*/
public stopMessaging(opts = { forceDestroy: false }): void {
if (
!opts?.forceDestroy &&
ActiveWidgetStore.instance.getWidgetPersistence(this.mockWidget.id, this.roomId ?? null)
) {
if (this.messaging === null || this.iframe === null) return;
if (opts.forceDestroy) {
// HACK: This is a really dirty way to ensure that Jitsi cleans up
// its hold on the webcam. Without this, the widget holds a media
// stream open, even after death. See https://github.com/vector-im/element-web/issues/7351
// In practice we could just do `+= ''` to trick the browser into
// thinking the URL changed, however I can foresee this being
// optimized out by a browser. Instead, we'll just point the iframe
// at a page that is reasonably safe to use in the event the iframe
// doesn't wink away.
this.iframe!.src = "about:blank";
} else if (ActiveWidgetStore.instance.getWidgetPersistence(this.mockWidget.id, this.roomId ?? null)) {
logger.log("Skipping destroy - persistent widget");
return;
}
if (!this.started) return;
WidgetMessagingStore.instance.stopMessaging(this.mockWidget, this.roomId);
this.messaging?.removeAllListeners(); // Guard against the 'ready' event firing after stopping
this.messaging = null;
this.iframe = null;
SdkContextClass.instance.roomViewStore.off(UPDATE_EVENT, this.onRoomViewStoreUpdate);