From 8f10ee01c6392c7d28d5586f235ee90df18e8419 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 11 Jul 2018 18:07:32 +0100 Subject: [PATCH 1/3] Implement always-on-screen capability for widgets As per https://github.com/matrix-org/matrix-doc/issues/1354 This is whitelisted to only jitsi widgets for now as per comment, mostly because any widget that we may make always-on-screen we need to preemptively put in a PersistedElement container, which is unnecessary for any other widget. Apologies that this does a bunch of refactoring which could have been split out separately: I only discovered what needed to be refactored in the process of doing this. Fixes https://github.com/vector-im/riot-web/issues/6984 --- res/css/views/rooms/_EventTile.scss | 1 - src/CallHandler.js | 3 +- src/FromWidgetPostMessageApi.js | 9 ++ src/components/views/elements/AppTile.js | 87 ++++++++++--------- .../views/elements/PersistedElement.js | 24 ++++- src/components/views/rooms/AppsDrawer.js | 43 +++++---- src/components/views/rooms/Stickerpicker.js | 17 +--- src/stores/ActiveWidgetStore.js | 84 ++++++++++++++++++ 8 files changed, 194 insertions(+), 74 deletions(-) create mode 100644 src/stores/ActiveWidgetStore.js diff --git a/res/css/views/rooms/_EventTile.scss b/res/css/views/rooms/_EventTile.scss index d95ed2ded0..737f22a0ac 100644 --- a/res/css/views/rooms/_EventTile.scss +++ b/res/css/views/rooms/_EventTile.scss @@ -31,7 +31,6 @@ limitations under the License. top: 14px; left: 8px; cursor: pointer; - z-index: 2; } .mx_EventTile.mx_EventTile_info .mx_EventTile_avatar { diff --git a/src/CallHandler.js b/src/CallHandler.js index 7403483e36..6f95c31175 100644 --- a/src/CallHandler.js +++ b/src/CallHandler.js @@ -444,7 +444,8 @@ function _startCallApp(roomId, type) { 'email=$matrix_user_id', ].join('&'); const widgetUrl = ( - 'https://scalar.vector.im/api/widgets' + + //'https://scalar.vector.im/api/widgets' + + 'http://localhost:8620' + '/jitsi.html?' + queryString ); diff --git a/src/FromWidgetPostMessageApi.js b/src/FromWidgetPostMessageApi.js index 792fd73733..ea7eeba756 100644 --- a/src/FromWidgetPostMessageApi.js +++ b/src/FromWidgetPostMessageApi.js @@ -18,6 +18,7 @@ import URL from 'url'; import dis from './dispatcher'; import IntegrationManager from './IntegrationManager'; import WidgetMessagingEndpoint from './WidgetMessagingEndpoint'; +import ActiveWidgetStore from './stores/ActiveWidgetStore'; const WIDGET_API_VERSION = '0.0.1'; // Current API version const SUPPORTED_WIDGET_API_VERSIONS = [ @@ -155,6 +156,14 @@ export default class FromWidgetPostMessageApi { const integType = (data && data.integType) ? data.integType : null; const integId = (data && data.integId) ? data.integId : null; IntegrationManager.open(integType, integId); + } else if (action === 'set_always_on_screen') { + // This is a new message: there is no reason to support the deprecated widgetData here + const data = event.data.data; + const val = data.value; + + if (ActiveWidgetStore.widgetHasCapability(widgetId, 'm.always_on_screen')) { + ActiveWidgetStore.setWidgetPersistence(widgetId, val); + } } else { console.warn('Widget postMessage event unhandled'); this.sendError(event, {message: 'The postMessage was unhandled'}); diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 7b69057e3e..cf69727b15 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -1,5 +1,6 @@ /** Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -33,6 +34,7 @@ import AppWarning from './AppWarning'; import MessageSpinner from './MessageSpinner'; import WidgetUtils from '../../../utils/WidgetUtils'; import dis from '../../../dispatcher'; +import ActiveWidgetStore from '../../../stores/ActiveWidgetStore'; const ALLOWED_APP_URL_SCHEMES = ['https:', 'http:']; const ENABLE_REACT_PERF = false; @@ -40,9 +42,13 @@ const ENABLE_REACT_PERF = false; export default class AppTile extends React.Component { constructor(props) { super(props); + + // The key used for PersistedElement + this._persistKey = 'widget_' + this.props.id; + this.state = this._getNewState(props); - this._onWidgetAction = this._onWidgetAction.bind(this); + this._onAction = this._onAction.bind(this); this._onMessage = this._onMessage.bind(this); this._onLoaded = this._onLoaded.bind(this); this._onEditClick = this._onEditClick.bind(this); @@ -50,7 +56,6 @@ export default class AppTile extends React.Component { this._onSnapshotClick = this._onSnapshotClick.bind(this); this.onClickMenuBar = this.onClickMenuBar.bind(this); this._onMinimiseClick = this._onMinimiseClick.bind(this); - this._onInitialLoad = this._onInitialLoad.bind(this); this._grantWidgetPermission = this._grantWidgetPermission.bind(this); this._revokeWidgetPermission = this._revokeWidgetPermission.bind(this); this._onPopoutWidgetClick = this._onPopoutWidgetClick.bind(this); @@ -66,9 +71,12 @@ export default class AppTile extends React.Component { _getNewState(newProps) { const widgetPermissionId = [newProps.room.roomId, encodeURIComponent(newProps.url)].join('_'); const hasPermissionToLoad = localStorage.getItem(widgetPermissionId); + + const PersistedElement = sdk.getComponent("elements.PersistedElement"); return { initialising: true, // True while we are mangling the widget URL - loading: this.props.waitForIframeLoad, // True while the iframe content is loading + // True while the iframe content is loading + loading: this.props.waitForIframeLoad && !PersistedElement.isMounted(this._persistKey), widgetUrl: this._addWurlParams(newProps.url), widgetPermissionId: widgetPermissionId, // Assume that widget has permission to load if we are the user who @@ -77,9 +85,6 @@ export default class AppTile extends React.Component { error: null, deleting: false, widgetPageTitle: newProps.widgetPageTitle, - allowedCapabilities: (this.props.whitelistCapabilities && this.props.whitelistCapabilities.length > 0) ? - this.props.whitelistCapabilities : [], - requestedCapabilities: [], }; } @@ -89,7 +94,7 @@ export default class AppTile extends React.Component { * @return {Boolean} True if capability supported */ _hasCapability(capability) { - return this.state.allowedCapabilities.some((c) => {return c === capability;}); + return ActiveWidgetStore.widgetHasCapability(this.props.id, capability); } /** @@ -142,30 +147,24 @@ export default class AppTile extends React.Component { window.addEventListener('message', this._onMessage, false); // Widget action listeners - this.dispatcherRef = dis.register(this._onWidgetAction); - } - - componentDidUpdate() { - // Allow parents to access widget messaging - if (this.props.collectWidgetMessaging) { - this.props.collectWidgetMessaging(this.widgetMessaging); - } + this.dispatcherRef = dis.register(this._onAction); } componentWillUnmount() { // Widget action listeners dis.unregister(this.dispatcherRef); - // Widget postMessage listeners - try { - if (this.widgetMessaging) { - this.widgetMessaging.stop(); - } - } catch (e) { - console.error('Failed to stop listening for widgetMessaging events', e.message); - } // Jitsi listener window.removeEventListener('message', this._onMessage); + + // if it's not remaining on screen, get rid of the PersistedElement container + if (!ActiveWidgetStore.getWidgetPersistence(this.props.id)) { + // FIXME: ActiveWidgetStore should probably worry about this? + const PersistedElement = sdk.getComponent("elements.PersistedElement"); + PersistedElement.destroyElement(this._persistKey); + ActiveWidgetStore.delWidgetMessaging(this.props.id); + ActiveWidgetStore.delWidgetCapabilities(this.props.id); + } } /** @@ -286,7 +285,7 @@ export default class AppTile extends React.Component { _onSnapshotClick(e) { console.warn("Requesting widget snapshot"); - this.widgetMessaging.getScreenshot() + ActiveWidgetStore.getWidgetMessaging(this.props.id).getScreenshot() .catch((err) => { console.error("Failed to get screenshot", err); }) @@ -341,19 +340,19 @@ export default class AppTile extends React.Component { * Called when widget iframe has finished loading */ _onLoaded() { - if (!this.widgetMessaging) { - this._onInitialLoad(); + if (!ActiveWidgetStore.getWidgetMessaging(this.props.id)) { + this._setupWidgetMessaging(); } this.setState({loading: false}); } - /** - * Called on initial load of the widget iframe - */ - _onInitialLoad() { - this.widgetMessaging = new WidgetMessaging(this.props.id, this.props.url, this.refs.appFrame.contentWindow); - this.widgetMessaging.getCapabilities().then((requestedCapabilities) => { - console.log(`Widget ${this.props.id} requested capabilities:`, requestedCapabilities); + _setupWidgetMessaging() { + // FIXME: There's probably no reason to do this here: it should probably be done entirely + // in ActiveWidgetStore. + const widgetMessaging = new WidgetMessaging(this.props.id, this.props.url, this.refs.appFrame.contentWindow); + ActiveWidgetStore.setWidgetMessaging(this.props.id, widgetMessaging); + widgetMessaging.getCapabilities().then((requestedCapabilities) => { + console.log(`Widget ${this.props.id} requested capabilities: ` + requestedCapabilities); requestedCapabilities = requestedCapabilities || []; // Allow whitelisted capabilities @@ -365,16 +364,15 @@ export default class AppTile extends React.Component { }, this.props.whitelistCapabilities); if (requestedWhitelistCapabilies.length > 0 ) { - console.warn(`Widget ${this.props.id} allowing requested, whitelisted properties:`, - requestedWhitelistCapabilies); + console.warn(`Widget ${this.props.id} allowing requested, whitelisted properties: ` + + requestedWhitelistCapabilies, + ); } } // TODO -- Add UI to warn about and optionally allow requested capabilities - this.setState({ - requestedCapabilities, - allowedCapabilities: this.state.allowedCapabilities.concat(requestedWhitelistCapabilies), - }); + + ActiveWidgetStore.setWidgetCapabilities(this.props.id, requestedWhitelistCapabilies); if (this.props.onCapabilityRequest) { this.props.onCapabilityRequest(requestedCapabilities); @@ -384,7 +382,7 @@ export default class AppTile extends React.Component { }); } - _onWidgetAction(payload) { + _onAction(payload) { if (payload.widgetId === this.props.id) { switch (payload.action) { case 'm.sticker': @@ -562,6 +560,15 @@ export default class AppTile extends React.Component { > ); + // if the widget would be allowed to remian on screen, we must put it in + // a PersistedElement from the get-go, otherwise the iframe will be + // re-mounted later when we do. + if (this.props.whitelistCapabilities.includes('m.always_on_screen')) { + const PersistedElement = sdk.getComponent("elements.PersistedElement"); + appTileBody = + {appTileBody} + ; + } } } else { const isRoomEncrypted = MatrixClientPeg.get().isRoomEncrypted(this.props.room.roomId); diff --git a/src/components/views/elements/PersistedElement.js b/src/components/views/elements/PersistedElement.js index 6816b4dab3..6f4eb09898 100644 --- a/src/components/views/elements/PersistedElement.js +++ b/src/components/views/elements/PersistedElement.js @@ -22,8 +22,12 @@ const PropTypes = require('prop-types'); // of doing reusable widgets like dialog boxes & menus where we go and // pass in a custom control as the actual body. +function getContainer(containerId) { + return document.getElementById(containerId); +} + function getOrCreateContainer(containerId) { - let container = document.getElementById(containerId); + let container = getContainer(containerId); if (!container) { container = document.createElement("div"); @@ -60,6 +64,24 @@ export default class PersistedElement extends React.Component { this.collectChild = this.collectChild.bind(this); } + /** + * Removes the DOM elements created when a PersistedElement with the given + * persistKey was mounted. The DOM elements will be re-added if another + * PeristedElement is mounted in the future. + * + * @param {string} persistKey Key used to uniquely identify this PersistedElement + */ + static destroyElement(persistKey) { + const container = getContainer('mx_persistedElement_' + persistKey); + if (container) { + container.remove(); + } + } + + static isMounted(persistKey) { + return Boolean(getContainer('mx_persistedElement_' + persistKey)); + } + collectChildContainer(ref) { this.childContainer = ref; } diff --git a/src/components/views/rooms/AppsDrawer.js b/src/components/views/rooms/AppsDrawer.js index 04e47f0f9f..da8c558cb5 100644 --- a/src/components/views/rooms/AppsDrawer.js +++ b/src/components/views/rooms/AppsDrawer.js @@ -1,5 +1,6 @@ /* Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -214,24 +215,30 @@ module.exports = React.createClass({ render: function() { const enableScreenshots = SettingsStore.getValue("enableWidgetScreenshots", this.props.room.room_id); - const apps = this.state.apps.map( - (app, index, arr) => { - return (); - }); + const apps = this.state.apps.map((app, index, arr) => { + const capWhitelist = enableScreenshots ? ["m.capability.screenshot"] : []; + + // Obviously anyone that can add a widget can claim it's a jitsi widget, + // so this doesn't really offer much over the set of domains we load + // widgets from at all, but it probably makes sense for sanity. + if (app.type == 'jitsi') capWhitelist.push("m.always_on_screen"); + + return (); + }); let addWidget; if (this.props.showApps && diff --git a/src/components/views/rooms/Stickerpicker.js b/src/components/views/rooms/Stickerpicker.js index d6afc28dad..45b89cbf62 100644 --- a/src/components/views/rooms/Stickerpicker.js +++ b/src/components/views/rooms/Stickerpicker.js @@ -24,6 +24,7 @@ import ScalarAuthClient from '../../../ScalarAuthClient'; import dis from '../../../dispatcher'; import AccessibleButton from '../elements/AccessibleButton'; import WidgetUtils from '../../../utils/WidgetUtils'; +import ActiveWidgetStore from '../../../stores/ActiveWidgetStore'; const widgetType = 'm.stickerpicker'; @@ -43,8 +44,6 @@ export default class Stickerpicker extends React.Component { this._onResize = this._onResize.bind(this); this._onFinished = this._onFinished.bind(this); - this._collectWidgetMessaging = this._collectWidgetMessaging.bind(this); - this.popoverWidth = 300; this.popoverHeight = 300; @@ -166,17 +165,10 @@ export default class Stickerpicker extends React.Component { ); } - _collectWidgetMessaging(widgetMessaging) { - this._appWidgetMessaging = widgetMessaging; - - // Do this now instead of in componentDidMount because we might not have had the - // reference to widgetMessaging when mounting - this._sendVisibilityToWidget(true); - } - _sendVisibilityToWidget(visible) { - if (this._appWidgetMessaging && visible !== this._prevSentVisibility) { - this._appWidgetMessaging.sendVisibility(visible); + const widgetMessaging = ActiveWidgetStore.getWidgetMessaging(this.state.stickerpickerWidget.id); + if (widgetMessaging && visible !== this._prevSentVisibility) { + widgetMessaging.sendVisibility(visible); this._prevSentVisibility = visible; } } @@ -217,7 +209,6 @@ export default class Stickerpicker extends React.Component { > Date: Wed, 11 Jul 2018 18:11:28 +0100 Subject: [PATCH 2/3] Obviously didn't mean to commit that change --- src/CallHandler.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/CallHandler.js b/src/CallHandler.js index 6f95c31175..7403483e36 100644 --- a/src/CallHandler.js +++ b/src/CallHandler.js @@ -444,8 +444,7 @@ function _startCallApp(roomId, type) { 'email=$matrix_user_id', ].join('&'); const widgetUrl = ( - //'https://scalar.vector.im/api/widgets' + - 'http://localhost:8620' + + 'https://scalar.vector.im/api/widgets' + '/jitsi.html?' + queryString ); From 5a5e967262b749140e610fb87fe784426a6892aa Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 12 Jul 2018 17:48:49 +0100 Subject: [PATCH 3/3] Fix avatars vanishing on hover Turns out the z-index was to make the avatar appear above the EventTile_line even though it comes before in the DOM (it's absolutely positioned to overlap with it). Instead, just put it afterwards in the DOM. --- src/components/views/rooms/EventTile.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index fff04d476d..0d8f2366ae 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -695,7 +695,6 @@ module.exports = withMatrixClient(React.createClass({
{ readAvatars }
- { avatar } { sender } + { + // The avatar goes after the event tile as it's absolutly positioned to be over the + // event tile line, so needs to be later in the DOM so it appears on top (this avoids + // the need for further z-indexing chaos) + } + { avatar } ); }