From 5f2d9b6c54a13850671674abcefada936ada6b6f Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 11 Apr 2018 18:20:40 +0100 Subject: [PATCH 1/7] Null check stylesheet href As commented Fixes https://github.com/vector-im/riot-web/issues/6489 --- src/Tinter.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Tinter.js b/src/Tinter.js index 7667e6d912..75a65412a4 100644 --- a/src/Tinter.js +++ b/src/Tinter.js @@ -326,7 +326,9 @@ class Tinter { // Vector Green as any other colour. // --matthew - if (ss.href && !ss.href.match(new RegExp('/theme-' + this.theme + '.css$'))) continue; + // stylesheets we don't have permission to access (eg. ones from extensions) have a null + // href and will throw exceptions if we try to access their rules. + if (!ss.href || !ss.href.match(new RegExp('/theme-' + this.theme + '.css$'))) continue; if (ss.disabled) continue; if (!ss.cssRules) continue; From 760f21b1d90a89cfae77a86e21c8469134487bc2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 11 Apr 2018 18:31:54 +0100 Subject: [PATCH 2/7] Put Tinter loop body in a try/catch So whatever other random ways this process fails in don't cause it to take out the whole app. --- src/Tinter.js | 96 ++++++++++++++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/src/Tinter.js b/src/Tinter.js index 75a65412a4..d24a4c3e74 100644 --- a/src/Tinter.js +++ b/src/Tinter.js @@ -298,58 +298,66 @@ class Tinter { for (let i = 0; i < document.styleSheets.length; i++) { const ss = document.styleSheets[i]; - if (!ss) continue; // well done safari >:( - // Chromium apparently sometimes returns null here; unsure why. - // see $14534907369972FRXBx:matrix.org in HQ - // ...ah, it's because there's a third party extension like - // privacybadger inserting its own stylesheet in there with a - // resource:// URI or something which results in a XSS error. - // See also #vector:matrix.org/$145357669685386ebCfr:matrix.org - // ...except some browsers apparently return stylesheets without - // hrefs, which we have no choice but ignore right now + try { + if (!ss) continue; // well done safari >:( + // Chromium apparently sometimes returns null here; unsure why. + // see $14534907369972FRXBx:matrix.org in HQ + // ...ah, it's because there's a third party extension like + // privacybadger inserting its own stylesheet in there with a + // resource:// URI or something which results in a XSS error. + // See also #vector:matrix.org/$145357669685386ebCfr:matrix.org + // ...except some browsers apparently return stylesheets without + // hrefs, which we have no choice but ignore right now - // XXX seriously? we are hardcoding the name of vector's CSS file in - // here? - // - // Why do we need to limit it to vector's CSS file anyway - if there - // are other CSS files affecting the doc don't we want to apply the - // same transformations to them? - // - // Iterating through the CSS looking for matches to hack on feels - // pretty horrible anyway. And what if the application skin doesn't use - // Vector Green as its primary color? - // --richvdh + // XXX seriously? we are hardcoding the name of vector's CSS file in + // here? + // + // Why do we need to limit it to vector's CSS file anyway - if there + // are other CSS files affecting the doc don't we want to apply the + // same transformations to them? + // + // Iterating through the CSS looking for matches to hack on feels + // pretty horrible anyway. And what if the application skin doesn't use + // Vector Green as its primary color? + // --richvdh - // Yes, tinting assumes that you are using the Riot skin for now. - // The right solution will be to move the CSS over to react-sdk. - // And yes, the default assets for the base skin might as well use - // Vector Green as any other colour. - // --matthew + // Yes, tinting assumes that you are using the Riot skin for now. + // The right solution will be to move the CSS over to react-sdk. + // And yes, the default assets for the base skin might as well use + // Vector Green as any other colour. + // --matthew - // stylesheets we don't have permission to access (eg. ones from extensions) have a null - // href and will throw exceptions if we try to access their rules. - if (!ss.href || !ss.href.match(new RegExp('/theme-' + this.theme + '.css$'))) continue; - if (ss.disabled) continue; - if (!ss.cssRules) continue; + // stylesheets we don't have permission to access (eg. ones from extensions) have a null + // href and will throw exceptions if we try to access their rules. + if (!ss.href || !ss.href.match(new RegExp('/theme-' + this.theme + '.css$'))) continue; + if (ss.disabled) continue; + if (!ss.cssRules) continue; - if (DEBUG) console.debug("calcCssFixups checking " + ss.cssRules.length + " rules for " + ss.href); + if (DEBUG) console.debug("calcCssFixups checking " + ss.cssRules.length + " rules for " + ss.href); - for (let j = 0; j < ss.cssRules.length; j++) { - const rule = ss.cssRules[j]; - if (!rule.style) continue; - if (rule.selectorText && rule.selectorText.match(/#mx_theme/)) continue; - for (let k = 0; k < this.cssAttrs.length; k++) { - const attr = this.cssAttrs[k]; - for (let l = 0; l < this.keyRgb.length; l++) { - if (rule.style[attr] === this.keyRgb[l]) { - this.cssFixups[this.theme].push({ - style: rule.style, - attr: attr, - index: l, - }); + for (let j = 0; j < ss.cssRules.length; j++) { + const rule = ss.cssRules[j]; + if (!rule.style) continue; + if (rule.selectorText && rule.selectorText.match(/#mx_theme/)) continue; + for (let k = 0; k < this.cssAttrs.length; k++) { + const attr = this.cssAttrs[k]; + for (let l = 0; l < this.keyRgb.length; l++) { + if (rule.style[attr] === this.keyRgb[l]) { + this.cssFixups[this.theme].push({ + style: rule.style, + attr: attr, + index: l, + }); + } } } } + } catch (e) { + // Catch any random exceptions that happen here: all sorts of things can go + // wrong with this (nulls, SecurityErrors) and mostly it's for other + // stylesheets that we don't want to proces anyway. We should not propagate an + // exception out since this will cause the app to fail to start. + console.log("Failed to calculate CSS fixups for a stylesheet: " + ss.href, e); } } if (DEBUG) { From 91a41392b4b62bee073df5e1f8f67ad3c7279d47 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 25 Dec 2017 14:25:13 -0700 Subject: [PATCH 3/7] Remove presence management The feature is incredibly buggy and doesn't work as expected due to server behaviour and client interaction. One of the major problems is the constantly confused presence state - this is caused by the mobile apps conflicting on the state of the web app, causing it to consider the user offline or online (and rarely away) depending on how riot-android/ios is behaving at the time. This reverts two PRs: * https://github.com/matrix-org/matrix-react-sdk/pull/1620 * https://github.com/matrix-org/matrix-react-sdk/pull/1482 The changes to the context menu positioning were not reverted as they are useful outside of presence management. Signed-off-by: Travis Ralston --- src/MatrixClientPeg.js | 1 - src/Presence.js | 36 +--- .../views/avatars/MemberPresenceAvatar.js | 169 ------------------ src/components/views/rooms/MessageComposer.js | 4 +- src/i18n/strings/en_EN.json | 1 + 5 files changed, 6 insertions(+), 205 deletions(-) delete mode 100644 src/components/views/avatars/MemberPresenceAvatar.js diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index 99841c986e..9d86a62de4 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -98,7 +98,6 @@ class MatrixClientPeg { const opts = utils.deepCopy(this.opts); // the react sdk doesn't work without this, so don't allow opts.pendingEventOrdering = "detached"; - opts.disablePresence = true; // we do this manually try { const promise = this.matrixClient.store.startup(); diff --git a/src/Presence.js b/src/Presence.js index fd9bcf516d..9367fe35cd 100644 --- a/src/Presence.js +++ b/src/Presence.js @@ -57,27 +57,13 @@ class Presence { return this.state; } - /** - * Get the current status message. - * @returns {String} the status message, may be null - */ - getStatusMessage() { - return this.statusMessage; - } - /** * Set the presence state. * If the state has changed, the Home Server will be notified. * @param {string} newState the new presence state (see PRESENCE enum) - * @param {String} statusMessage an optional status message for the presence - * @param {boolean} maintain true to have this status maintained by this tracker */ - setState(newState, statusMessage=null, maintain=false) { - if (this.maintain) { - // Don't update presence if we're maintaining a particular status - return; - } - if (newState === this.state && statusMessage === this.statusMessage) { + setState(newState) { + if (newState === this.state) { return; } if (PRESENCE_STATES.indexOf(newState) === -1) { @@ -87,37 +73,21 @@ class Presence { return; } const old_state = this.state; - const old_message = this.statusMessage; this.state = newState; - this.statusMessage = statusMessage; - this.maintain = maintain; if (MatrixClientPeg.get().isGuest()) { return; // don't try to set presence when a guest; it won't work. } - const updateContent = { - presence: this.state, - status_msg: this.statusMessage ? this.statusMessage : '', - }; - const self = this; - MatrixClientPeg.get().setPresence(updateContent).done(function() { + MatrixClientPeg.get().setPresence(this.state).done(function() { console.log("Presence: %s", newState); - - // We have to dispatch because the js-sdk is unreliable at telling us about our own presence - dis.dispatch({action: "self_presence_updated", statusInfo: updateContent}); }, function(err) { console.error("Failed to set presence: %s", err); self.state = old_state; - self.statusMessage = old_message; }); } - stopMaintainingStatus() { - this.maintain = false; - } - /** * Callback called when the user made no action on the page for UNAVAILABLE_TIME ms. * @private diff --git a/src/components/views/avatars/MemberPresenceAvatar.js b/src/components/views/avatars/MemberPresenceAvatar.js deleted file mode 100644 index aa6def00ae..0000000000 --- a/src/components/views/avatars/MemberPresenceAvatar.js +++ /dev/null @@ -1,169 +0,0 @@ -/* - Copyright 2017 Travis Ralston - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ - -'use strict'; - -import React from "react"; -import PropTypes from 'prop-types'; -import * as sdk from "../../../index"; -import MatrixClientPeg from "../../../MatrixClientPeg"; -import AccessibleButton from '../elements/AccessibleButton'; -import Presence from "../../../Presence"; -import dispatcher from "../../../dispatcher"; -import * as ContextualMenu from "../../structures/ContextualMenu"; -import SettingsStore from "../../../settings/SettingsStore"; - -// This is an avatar with presence information and controls on it. -module.exports = React.createClass({ - displayName: 'MemberPresenceAvatar', - - propTypes: { - member: PropTypes.object.isRequired, - width: PropTypes.number, - height: PropTypes.number, - resizeMethod: PropTypes.string, - }, - - getDefaultProps: function() { - return { - width: 40, - height: 40, - resizeMethod: 'crop', - }; - }, - - getInitialState: function() { - let presenceState = null; - let presenceMessage = null; - - // RoomMembers do not necessarily have a user. - if (this.props.member.user) { - presenceState = this.props.member.user.presence; - presenceMessage = this.props.member.user.presenceStatusMsg; - } - - return { - status: presenceState, - message: presenceMessage, - }; - }, - - componentWillMount: function() { - MatrixClientPeg.get().on("User.presence", this.onUserPresence); - this.dispatcherRef = dispatcher.register(this.onAction); - }, - - componentWillUnmount: function() { - if (MatrixClientPeg.get()) { - MatrixClientPeg.get().removeListener("User.presence", this.onUserPresence); - } - dispatcher.unregister(this.dispatcherRef); - }, - - onAction: function(payload) { - if (payload.action !== "self_presence_updated") return; - if (this.props.member.userId !== MatrixClientPeg.get().getUserId()) return; - this.setState({ - status: payload.statusInfo.presence, - message: payload.statusInfo.status_msg, - }); - }, - - onUserPresence: function(event, user) { - if (user.userId !== MatrixClientPeg.get().getUserId()) return; - this.setState({ - status: user.presence, - message: user.presenceStatusMsg, - }); - }, - - onStatusChange: function(newStatus) { - Presence.stopMaintainingStatus(); - if (newStatus === "online") { - Presence.setState(newStatus); - } else Presence.setState(newStatus, null, true); - }, - - onClick: function(e) { - const PresenceContextMenu = sdk.getComponent('context_menus.PresenceContextMenu'); - const elementRect = e.target.getBoundingClientRect(); - - // The window X and Y offsets are to adjust position when zoomed in to page - const x = (elementRect.left + window.pageXOffset) - (elementRect.width / 2) + 3; - const chevronOffset = 12; - let y = elementRect.top + (elementRect.height / 2) + window.pageYOffset; - y = y - (chevronOffset + 4); // where 4 is 1/4 the height of the chevron - - ContextualMenu.createMenu(PresenceContextMenu, { - chevronOffset: chevronOffset, - chevronFace: 'bottom', - left: x, - top: y, - menuWidth: 125, - currentStatus: this.state.status, - onChange: this.onStatusChange, - }); - - e.stopPropagation(); - - // XXX NB the following assumes that user is non-null, which is not valid - // const presenceState = this.props.member.user.presence; - // const presenceLastActiveAgo = this.props.member.user.lastActiveAgo; - // const presenceLastTs = this.props.member.user.lastPresenceTs; - // const presenceCurrentlyActive = this.props.member.user.currentlyActive; - // const presenceMessage = this.props.member.user.presenceStatusMsg; - }, - - render: function() { - const MemberAvatar = sdk.getComponent("avatars.MemberAvatar"); - - let onClickFn = null; - if (this.props.member.userId === MatrixClientPeg.get().getUserId()) { - onClickFn = this.onClick; - } - - const avatarNode = ( - - ); - let statusNode = ( - - ); - - // LABS: Disable presence management functions for now - // Also disable the presence information if there's no status information - if (!SettingsStore.isFeatureEnabled("feature_presence_management") || !this.state.status) { - statusNode = null; - onClickFn = null; - } - - let avatar = ( -
- { avatarNode } - { statusNode } -
- ); - if (onClickFn) { - avatar = ( - - { avatarNode } - { statusNode } - - ); - } - return avatar; - }, -}); diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index 6ad033fa0c..7a55df11e0 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -251,7 +251,7 @@ export default class MessageComposer extends React.Component { render() { const me = this.props.room.getMember(MatrixClientPeg.get().credentials.userId); const uploadInputStyle = {display: 'none'}; - const MemberPresenceAvatar = sdk.getComponent('avatars.MemberPresenceAvatar'); + const MemberAvatar = sdk.getComponent('avatars.MemberAvatar'); const TintableSvg = sdk.getComponent("elements.TintableSvg"); const MessageComposerInput = sdk.getComponent("rooms.MessageComposerInput"); @@ -259,7 +259,7 @@ export default class MessageComposer extends React.Component { controls.push(
- +
, ); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 84a65fa3cf..73a97e28e4 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -170,6 +170,7 @@ "%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s", "%(widgetName)s widget added by %(senderName)s": "%(widgetName)s widget added by %(senderName)s", "%(widgetName)s widget removed by %(senderName)s": "%(widgetName)s widget removed by %(senderName)s", + "Message Pinning": "Message Pinning", "%(displayName)s is typing": "%(displayName)s is typing", "%(names)s and %(count)s others are typing|other": "%(names)s and %(count)s others are typing", "%(names)s and %(count)s others are typing|one": "%(names)s and one other is typing", From 3f069af0670c4df445d8e833c9a96d3ed09315d3 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 25 Dec 2017 14:33:46 -0700 Subject: [PATCH 4/7] Also remove the setting Signed-off-by: Travis Ralston --- src/settings/Settings.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/settings/Settings.js b/src/settings/Settings.js index e33aed1a6c..c88a68302f 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -88,12 +88,6 @@ export const SETTINGS = { supportedLevels: LEVELS_FEATURE, default: false, }, - "feature_presence_management": { - isFeature: true, - displayName: _td("Presence Management"), - supportedLevels: LEVELS_FEATURE, - default: false, - }, "MessageComposerInput.dontSuggestEmoji": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, displayName: _td('Disable Emoji suggestions while typing'), From f38bb1f2c2b5b0f459dcffcc7baf0d373dfe296b Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 12 Apr 2018 12:40:33 +0100 Subject: [PATCH 5/7] Bump js-sdk --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 95c899fedc..d17e1ae42d 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "isomorphic-fetch": "^2.2.1", "linkifyjs": "^2.1.3", "lodash": "^4.13.1", - "matrix-js-sdk": "0.10.0", + "matrix-js-sdk": "0.10.1", "optimist": "^0.6.1", "prop-types": "^15.5.8", "querystring": "^0.2.0", From 4a1d0fc8ab91e0ca2345acb5e0e5418f5992805c Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 12 Apr 2018 12:50:51 +0100 Subject: [PATCH 6/7] Prepare changelog for v0.12.2 --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a74351aa69..1196b05599 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,16 @@ +Changes in [0.12.2](https://github.com/matrix-org/matrix-react-sdk/releases/tag/v0.12.2) (2018-04-12) +===================================================================================================== +[Full Changelog](https://github.com/matrix-org/matrix-react-sdk/compare/v0.12.1...v0.12.2) + + * Null check stylesheet href + [\#1835](https://github.com/matrix-org/matrix-react-sdk/pull/1835) + * Remove the presence management labs feature + Changes in [0.12.1](https://github.com/matrix-org/matrix-react-sdk/releases/tag/v0.12.1) (2018-04-11) ===================================================================================================== [Full Changelog](https://github.com/matrix-org/matrix-react-sdk/compare/v0.12.0...v0.12.1) + * Use correct js-sdk version Changes in [0.12.0](https://github.com/matrix-org/matrix-react-sdk/releases/tag/v0.12.0) (2018-04-11) ===================================================================================================== From c06fc31bb2ef5fc4468431a77348e017870b882b Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 12 Apr 2018 12:50:52 +0100 Subject: [PATCH 7/7] v0.12.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d17e1ae42d..1f28622712 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "matrix-react-sdk", - "version": "0.12.1", + "version": "0.12.2", "description": "SDK for matrix.org using React", "author": "matrix.org", "repository": {