From b185eed46256edbfc1f287424f5f027dd4e38812 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 18 Nov 2019 17:56:33 -0700 Subject: [PATCH 1/7] Wire up the widget permission prompt to the cross-platform setting This doesn't have any backwards compatibility with anyone who has already clicked "Allow". We kinda want everyone to read the new prompt, so what better way to do it than effectively revoke all widget permissions? Part of https://github.com/vector-im/riot-web/issues/11262 --- src/components/views/elements/AppTile.js | 53 ++++++++++++------- .../views/elements/PersistentApp.js | 3 +- src/components/views/rooms/AppsDrawer.js | 3 +- src/utils/WidgetUtils.js | 3 +- 4 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index ffd9d73cca..db5978c792 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -34,7 +34,7 @@ import dis from '../../../dispatcher'; import ActiveWidgetStore from '../../../stores/ActiveWidgetStore'; import classNames from 'classnames'; import {IntegrationManagers} from "../../../integrations/IntegrationManagers"; -import SettingsStore from "../../../settings/SettingsStore"; +import SettingsStore, {SettingLevel} from "../../../settings/SettingsStore"; const ALLOWED_APP_URL_SCHEMES = ['https:', 'http:']; const ENABLE_REACT_PERF = false; @@ -69,8 +69,11 @@ export default class AppTile extends React.Component { * @return {Object} Updated component state to be set with setState */ _getNewState(newProps) { - const widgetPermissionId = [newProps.room.roomId, encodeURIComponent(newProps.url)].join('_'); - const hasPermissionToLoad = localStorage.getItem(widgetPermissionId); + // This is a function to make the impact of calling SettingsStore slightly less + const hasPermissionToLoad = () => { + const currentlyAllowedWidgets = SettingsStore.getValue("allowedWidgets", newProps.room.roomId); + return !!currentlyAllowedWidgets[newProps.eventId]; + }; const PersistedElement = sdk.getComponent("elements.PersistedElement"); return { @@ -78,10 +81,9 @@ export default class AppTile extends React.Component { // 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 // added it to the room, or if explicitly granted by the user - hasPermissionToLoad: hasPermissionToLoad === 'true' || newProps.userId === newProps.creatorUserId, + hasPermissionToLoad: newProps.userId === newProps.creatorUserId || hasPermissionToLoad(), error: null, deleting: false, widgetPageTitle: newProps.widgetPageTitle, @@ -446,24 +448,38 @@ export default class AppTile extends React.Component { }); } - /* TODO -- Store permission in account data so that it is persisted across multiple devices */ _grantWidgetPermission() { - console.warn('Granting permission to load widget - ', this.state.widgetUrl); - localStorage.setItem(this.state.widgetPermissionId, true); - this.setState({hasPermissionToLoad: true}); - // Now that we have permission, fetch the IM token - this.setScalarToken(); + const roomId = this.props.room.roomId; + console.info("Granting permission for widget to load: " + this.props.eventId); + const current = SettingsStore.getValue("allowedWidgets", roomId); + current[this.props.eventId] = true; + SettingsStore.setValue("allowedWidgets", roomId, SettingLevel.ROOM_ACCOUNT, current).then(() => { + this.setState({hasPermissionToLoad: true}); + + // Fetch a token for the integration manager, now that we're allowed to + this.setScalarToken(); + }).catch(err => { + console.error(err); + // We don't really need to do anything about this - the user will just hit the button again. + }); } _revokeWidgetPermission() { - console.warn('Revoking permission to load widget - ', this.state.widgetUrl); - localStorage.removeItem(this.state.widgetPermissionId); - this.setState({hasPermissionToLoad: false}); + const roomId = this.props.room.roomId; + console.info("Revoking permission for widget to load: " + this.props.eventId); + const current = SettingsStore.getValue("allowedWidgets", roomId); + current[this.props.eventId] = false; + SettingsStore.setValue("allowedWidgets", roomId, SettingLevel.ROOM_ACCOUNT, current).then(() => { + this.setState({hasPermissionToLoad: false}); - // Force the widget to be non-persistent - ActiveWidgetStore.destroyPersistentWidget(this.props.id); - const PersistedElement = sdk.getComponent("elements.PersistedElement"); - PersistedElement.destroyElement(this._persistKey); + // Force the widget to be non-persistent (able to be deleted/forgotten) + ActiveWidgetStore.destroyPersistentWidget(this.props.id); + const PersistedElement = sdk.getComponent("elements.PersistedElement"); + PersistedElement.destroyElement(this._persistKey); + }).catch(err => { + console.error(err); + // We don't really need to do anything about this - the user will just hit the button again. + }); } formatAppTileName() { @@ -720,6 +736,7 @@ AppTile.displayName ='AppTile'; AppTile.propTypes = { id: PropTypes.string.isRequired, + eventId: PropTypes.string, // required for room widgets url: PropTypes.string.isRequired, name: PropTypes.string.isRequired, room: PropTypes.object.isRequired, diff --git a/src/components/views/elements/PersistentApp.js b/src/components/views/elements/PersistentApp.js index 391e7728f6..47783a45c3 100644 --- a/src/components/views/elements/PersistentApp.js +++ b/src/components/views/elements/PersistentApp.js @@ -67,13 +67,14 @@ module.exports = createReactClass({ return ev.getStateKey() === ActiveWidgetStore.getPersistentWidgetId(); }); const app = WidgetUtils.makeAppConfig( - appEvent.getStateKey(), appEvent.getContent(), appEvent.getSender(), persistentWidgetInRoomId, + appEvent.getStateKey(), appEvent.getContent(), appEvent.getSender(), persistentWidgetInRoomId, appEvent.getId(), ); const capWhitelist = WidgetUtils.getCapWhitelistForAppTypeInRoomId(app.type, persistentWidgetInRoomId); const AppTile = sdk.getComponent('elements.AppTile'); return { - return WidgetUtils.makeAppConfig(ev.getStateKey(), ev.getContent(), ev.getSender()); + return WidgetUtils.makeAppConfig(ev.getStateKey(), ev.getContent(), ev.getSender(), ev.getRoomId(), ev.getId()); }); }, @@ -159,6 +159,7 @@ module.exports = createReactClass({ return ( Date: Mon, 18 Nov 2019 18:02:47 -0700 Subject: [PATCH 2/7] Appease the linter --- src/components/views/elements/PersistentApp.js | 3 ++- src/components/views/rooms/AppsDrawer.js | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/views/elements/PersistentApp.js b/src/components/views/elements/PersistentApp.js index 47783a45c3..19e4be6083 100644 --- a/src/components/views/elements/PersistentApp.js +++ b/src/components/views/elements/PersistentApp.js @@ -67,7 +67,8 @@ module.exports = createReactClass({ return ev.getStateKey() === ActiveWidgetStore.getPersistentWidgetId(); }); const app = WidgetUtils.makeAppConfig( - appEvent.getStateKey(), appEvent.getContent(), appEvent.getSender(), persistentWidgetInRoomId, appEvent.getId(), + appEvent.getStateKey(), appEvent.getContent(), appEvent.getSender(), + persistentWidgetInRoomId, appEvent.getId(), ); const capWhitelist = WidgetUtils.getCapWhitelistForAppTypeInRoomId(app.type, persistentWidgetInRoomId); const AppTile = sdk.getComponent('elements.AppTile'); diff --git a/src/components/views/rooms/AppsDrawer.js b/src/components/views/rooms/AppsDrawer.js index 618536ef7c..e53570dc5b 100644 --- a/src/components/views/rooms/AppsDrawer.js +++ b/src/components/views/rooms/AppsDrawer.js @@ -107,7 +107,9 @@ module.exports = createReactClass({ this.props.room.roomId, WidgetUtils.getRoomWidgets(this.props.room), ); return widgets.map((ev) => { - return WidgetUtils.makeAppConfig(ev.getStateKey(), ev.getContent(), ev.getSender(), ev.getRoomId(), ev.getId()); + return WidgetUtils.makeAppConfig( + ev.getStateKey(), ev.getContent(), ev.getSender(), ev.getRoomId(), ev.getId(), + ); }); }, From 5a700b518a5063a1484ee339daf2a4deb611d485 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 13:41:06 +0000 Subject: [PATCH 3/7] Get theme automatically from system setting Uses CSS `prefers-color-scheme` to get the user's preferred colour scheme. Also bundles up some theme logic into its own class. --- src/components/structures/MatrixChat.js | 17 ++--- .../tabs/user/GeneralUserSettingsTab.js | 28 ++++++-- src/i18n/strings/en_EN.json | 1 + src/settings/Settings.js | 5 ++ src/theme.js | 67 ++++++++++++++++++- 5 files changed, 100 insertions(+), 18 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index c6efb56a9d..661a0c7077 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -59,7 +59,7 @@ import { ValidatedServerConfig } from "../../utils/AutoDiscoveryUtils"; import AutoDiscoveryUtils from "../../utils/AutoDiscoveryUtils"; import DMRoomMap from '../../utils/DMRoomMap'; import { countRoomsWithNotif } from '../../RoomNotifs'; -import { setTheme } from "../../theme"; +import { ThemeWatcher } from "../../theme"; import { storeRoomAliasInCache } from '../../RoomAliasCache'; import { defer } from "../../utils/promise"; @@ -274,7 +274,8 @@ export default createReactClass({ componentDidMount: function() { this.dispatcherRef = dis.register(this.onAction); - this._themeWatchRef = SettingsStore.watchSetting("theme", null, this._onThemeChanged); + this._themeWatcher = new ThemeWatcher(); + this._themeWatcher.start(); this.focusComposer = false; @@ -361,7 +362,7 @@ export default createReactClass({ componentWillUnmount: function() { Lifecycle.stopMatrixClient(); dis.unregister(this.dispatcherRef); - SettingsStore.unwatchSetting(this._themeWatchRef); + this._themeWatcher.stop(); window.removeEventListener("focus", this.onFocus); window.removeEventListener('resize', this.handleResize); this.state.resizeNotifier.removeListener("middlePanelResized", this._dispatchTimelineResize); @@ -384,13 +385,6 @@ export default createReactClass({ } }, - _onThemeChanged: function(settingName, roomId, atLevel, newValue) { - dis.dispatch({ - action: 'set_theme', - value: newValue, - }); - }, - startPageChangeTimer() { // Tor doesn't support performance if (!performance || !performance.mark) return null; @@ -672,9 +666,6 @@ export default createReactClass({ }); break; } - case 'set_theme': - setTheme(payload.value); - break; case 'on_logging_in': // We are now logging in, so set the state to reflect that // NB. This does not touch 'ready' since if our dispatches diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js index d400e7a839..50f37cea1f 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js @@ -27,7 +27,7 @@ import LanguageDropdown from "../../../elements/LanguageDropdown"; import AccessibleButton from "../../../elements/AccessibleButton"; import DeactivateAccountDialog from "../../../dialogs/DeactivateAccountDialog"; import PropTypes from "prop-types"; -import {enumerateThemes} from "../../../../../theme"; +import {enumerateThemes, ThemeWatcher} from "../../../../../theme"; import PlatformPeg from "../../../../../PlatformPeg"; import MatrixClientPeg from "../../../../../MatrixClientPeg"; import sdk from "../../../../.."; @@ -50,6 +50,7 @@ export default class GeneralUserSettingsTab extends React.Component { this.state = { language: languageHandler.getCurrentLanguage(), theme: SettingsStore.getValueAt(SettingLevel.ACCOUNT, "theme"), + useSystemTheme: SettingsStore.getValueAt(SettingLevel.DEVICE, "use_system_theme"), haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl()), serverSupportsSeparateAddAndBind: null, idServerHasUnsignedTerms: false, @@ -177,16 +178,22 @@ export default class GeneralUserSettingsTab extends React.Component { // so remember what the value was before we tried to set it so we can revert const oldTheme = SettingsStore.getValue('theme'); SettingsStore.setValue("theme", null, SettingLevel.ACCOUNT, newTheme).catch(() => { - dis.dispatch({action: 'set_theme', value: oldTheme}); + dis.dispatch({action: 'recheck_theme'}); this.setState({theme: oldTheme}); }); this.setState({theme: newTheme}); // The settings watcher doesn't fire until the echo comes back from the // server, so to make the theme change immediately we need to manually // do the dispatch now - dis.dispatch({action: 'set_theme', value: newTheme}); + dis.dispatch({action: 'recheck_theme'}); }; + _onUseSystemThemeChanged = (checked) => { + this.setState({useSystemTheme: checked}); + dis.dispatch({action: 'recheck_theme'}); + } + + _onPasswordChangeError = (err) => { // TODO: Figure out a design that doesn't involve replacing the current dialog let errMsg = err.error || ""; @@ -297,11 +304,24 @@ export default class GeneralUserSettingsTab extends React.Component { _renderThemeSection() { const SettingsFlag = sdk.getComponent("views.elements.SettingsFlag"); + + const themeWatcher = new ThemeWatcher(); + let systemThemeSection; + if (themeWatcher.isSystemThemeSupported()) { + systemThemeSection =
+ +
; + } return (
{_t("Theme")} + {systemThemeSection} + value={this.state.theme} onChange={this._onThemeChange} + disabled={this.state.useSystemTheme} + > {Object.entries(enumerateThemes()).map(([theme, text]) => { return ; })} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 473efdfb76..5dfcd038ec 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -363,6 +363,7 @@ "Automatically replace plain text Emoji": "Automatically replace plain text Emoji", "Mirror local video feed": "Mirror local video feed", "Enable Community Filter Panel": "Enable Community Filter Panel", + "Match system theme": "Match system theme", "Allow Peer-to-Peer for 1:1 calls": "Allow Peer-to-Peer for 1:1 calls", "Send analytics data": "Send analytics data", "Never send encrypted messages to unverified devices from this device": "Never send encrypted messages to unverified devices from this device", diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 718a0daec3..8a3bc3ecbc 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -275,6 +275,11 @@ export const SETTINGS = { supportedLevels: LEVELS_ACCOUNT_SETTINGS, default: [], }, + "use_system_theme": { + supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, + default: true, + displayName: _td("Match system theme"), + }, "webRtcAllowPeerToPeer": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG, displayName: _td('Allow Peer-to-Peer for 1:1 calls'), diff --git a/src/theme.js b/src/theme.js index 8a15c606d7..8996fe28fd 100644 --- a/src/theme.js +++ b/src/theme.js @@ -19,8 +19,72 @@ import {_t} from "./languageHandler"; export const DEFAULT_THEME = "light"; import Tinter from "./Tinter"; +import dis from "./dispatcher"; import SettingsStore from "./settings/SettingsStore"; +export class ThemeWatcher { + static _instance = null; + + constructor() { + this._themeWatchRef = null; + this._systemThemeWatchRef = null; + this._dispatcherRef = null; + + // we have both here as each may either match or not match, so by having both + // we can get the tristate of dark/light/unsupported + this._preferDark = global.matchMedia("(prefers-color-scheme: dark)"); + this._preferLight = global.matchMedia("(prefers-color-scheme: light)"); + + this._currentTheme = this.getEffectiveTheme(); + } + + start() { + this._themeWatchRef = SettingsStore.watchSetting("theme", null, this._onChange); + this._systemThemeWatchRef = SettingsStore.watchSetting("use_system_theme", null, this._onChange); + this._preferDark.addEventListener('change', this._onChange); + this._preferLight.addEventListener('change', this._onChange); + this._dispatcherRef = dis.register(this._onAction); + } + + stop() { + this._preferDark.removeEventListener('change', this._onChange); + this._preferLight.removeEventListener('change', this._onChange); + SettingsStore.unwatchSetting(this._systemThemeWatchRef); + SettingsStore.unwatchSetting(this._themeWatchRef); + dis.unregister(this._dispatcherRef); + } + + _onChange = () => { + this.recheck(); + } + + _onAction = (payload) => { + if (payload.action === 'recheck_theme') { + this.recheck(); + } + } + + recheck() { + const oldTheme = this._currentTheme; + this._currentTheme = this.getEffectiveTheme(); + if (oldTheme !== this._currentTheme) { + setTheme(this._currentTheme); + } + } + + getEffectiveTheme() { + if (SettingsStore.getValue('use_system_theme')) { + if (this._preferDark.matches) return 'dark'; + if (this._preferLight.matches) return 'light'; + } + return SettingsStore.getValue('theme'); + } + + isSystemThemeSupported() { + return this._preferDark || this._preferLight; + } +} + export function enumerateThemes() { const BUILTIN_THEMES = { "light": _t("Light theme"), @@ -83,7 +147,8 @@ export function getBaseTheme(theme) { */ export function setTheme(theme) { if (!theme) { - theme = SettingsStore.getValue("theme"); + const themeWatcher = new ThemeWatcher(); + theme = themeWatcher.getEffectiveTheme(); } let stylesheetName = theme; if (theme.startsWith("custom-")) { From 71f5c8b2b045a85beb563be5ef5483a6c0137723 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 13:47:54 +0000 Subject: [PATCH 4/7] Lint --- .../views/settings/tabs/user/GeneralUserSettingsTab.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js index 50f37cea1f..dbe0a9a301 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js @@ -188,7 +188,7 @@ export default class GeneralUserSettingsTab extends React.Component { dis.dispatch({action: 'recheck_theme'}); }; - _onUseSystemThemeChanged = (checked) => { + _onUseSystemThemeChanged = (checked) => { this.setState({useSystemTheme: checked}); dis.dispatch({action: 'recheck_theme'}); } From a7444152213fc18e070e9e3ffca92f349c51fb47 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 15:34:32 +0000 Subject: [PATCH 5/7] Add hack to work around mystery settings bug --- .../views/settings/tabs/user/GeneralUserSettingsTab.js | 5 ++++- src/theme.js | 8 +++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js index dbe0a9a301..b518f7c81b 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js @@ -185,7 +185,10 @@ export default class GeneralUserSettingsTab extends React.Component { // The settings watcher doesn't fire until the echo comes back from the // server, so to make the theme change immediately we need to manually // do the dispatch now - dis.dispatch({action: 'recheck_theme'}); + // XXX: The local echoed value appears to be unreliable, in particular + // when settings custom themes(!) so adding forceTheme to override + // the value from settings. + dis.dispatch({action: 'recheck_theme', forceTheme: newTheme}); }; _onUseSystemThemeChanged = (checked) => { diff --git a/src/theme.js b/src/theme.js index 8996fe28fd..5e390bf2c8 100644 --- a/src/theme.js +++ b/src/theme.js @@ -60,13 +60,15 @@ export class ThemeWatcher { _onAction = (payload) => { if (payload.action === 'recheck_theme') { - this.recheck(); + // XXX forceTheme + this.recheck(payload.forceTheme); } } - recheck() { + // XXX: forceTheme param aded here as local echo appears to be unreliable + recheck(forceTheme) { const oldTheme = this._currentTheme; - this._currentTheme = this.getEffectiveTheme(); + this._currentTheme = forceTheme === undefined ? this.getEffectiveTheme() : forceTheme; if (oldTheme !== this._currentTheme) { setTheme(this._currentTheme); } From 518130c912dfd8cf196be96698fc4d342b79841e Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 15:37:48 +0000 Subject: [PATCH 6/7] add bug link --- src/theme.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/theme.js b/src/theme.js index 5e390bf2c8..43bc813d34 100644 --- a/src/theme.js +++ b/src/theme.js @@ -66,6 +66,7 @@ export class ThemeWatcher { } // XXX: forceTheme param aded here as local echo appears to be unreliable + // https://github.com/vector-im/riot-web/issues/11443 recheck(forceTheme) { const oldTheme = this._currentTheme; this._currentTheme = forceTheme === undefined ? this.getEffectiveTheme() : forceTheme; From e36f4375b0bdece26b729679c61e530ca17a26bf Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Nov 2019 16:12:14 +0000 Subject: [PATCH 7/7] Bugfix & clearer setting name --- src/i18n/strings/en_EN.json | 2 +- src/settings/Settings.js | 2 +- src/theme.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 5dfcd038ec..c62b39cda0 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -363,7 +363,7 @@ "Automatically replace plain text Emoji": "Automatically replace plain text Emoji", "Mirror local video feed": "Mirror local video feed", "Enable Community Filter Panel": "Enable Community Filter Panel", - "Match system theme": "Match system theme", + "Match system dark mode setting": "Match system dark mode setting", "Allow Peer-to-Peer for 1:1 calls": "Allow Peer-to-Peer for 1:1 calls", "Send analytics data": "Send analytics data", "Never send encrypted messages to unverified devices from this device": "Never send encrypted messages to unverified devices from this device", diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 8a3bc3ecbc..59e60353b8 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -278,7 +278,7 @@ export const SETTINGS = { "use_system_theme": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, default: true, - displayName: _td("Match system theme"), + displayName: _td("Match system dark mode setting"), }, "webRtcAllowPeerToPeer": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG, diff --git a/src/theme.js b/src/theme.js index 43bc813d34..fa7e3f783b 100644 --- a/src/theme.js +++ b/src/theme.js @@ -84,7 +84,7 @@ export class ThemeWatcher { } isSystemThemeSupported() { - return this._preferDark || this._preferLight; + return this._preferDark.matches || this._preferLight.matches; } }