From 7662b5ac8f5d8db849730a0e0c74b4535aaf6c13 Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Fri, 27 Oct 2017 13:47:51 +0100 Subject: [PATCH 01/26] Unfinished, non-working changes to try and handle URL changes gracefully --- src/components/views/elements/AppTile.js | 26 +++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 0070af1fb29..7da0407c002 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -99,14 +99,27 @@ export default React.createClass({ }, componentWillMount: function() { + window.addEventListener('message', this._onMessage, false); + this.updateWidgetContent(); + }, + + // Update widget content + updateWidgetContent() { + this.setScalarToken(); + this.setState({ + loading: true, + }); + }, + + // Adds a scalar token to the widget URL, if required + setScalarToken() { if (!this.isScalarUrl()) { return; } // Fetch the token before loading the iframe as we need to mangle the URL - this.setState({ - loading: true, - }); - this._scalarClient = new ScalarAuthClient(); + if(! this._scalarClient) { + this._scalarClient = new ScalarAuthClient(); + } this._scalarClient.getScalarToken().done((token) => { // Append scalar_token as a query param this._scalarClient.scalarToken = token; @@ -128,13 +141,16 @@ export default React.createClass({ loading: false, }); }); - window.addEventListener('message', this._onMessage, false); }, componentWillUnmount() { window.removeEventListener('message', this._onMessage); }, + componentWillReceiveProps(nextProps) { + console.warn("Apptile", this.id, "got new props", this.url, nextProps.url); + }, + _onMessage(event) { if (this.props.type !== 'jitsi') { return; From 0a7273bf1df7ee1c3216288da8003bc43debd9cc Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Fri, 27 Oct 2017 16:38:49 +0100 Subject: [PATCH 02/26] Add URL search paramas polyfill --- package-lock.json | 5 +++++ package.json | 1 + 2 files changed, 6 insertions(+) diff --git a/package-lock.json b/package-lock.json index 6dd02674be2..5810e571f19 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6241,6 +6241,11 @@ } } }, + "url-search-params": { + "version": "0.10.0", + "resolved": "https://registry.npmjs.org/url-search-params/-/url-search-params-0.10.0.tgz", + "integrity": "sha512-oFPzmbPAbdthStgffGq8alULe47skPDt1X3KW6NOQnKkcLHP4IS1NfdfHG/CBP5lGsr2gDzNp87pfWLx/eIxjw==" + }, "user-home": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/user-home/-/user-home-1.1.1.tgz", diff --git a/package.json b/package.json index 883fdae8d5d..9c3dddf346f 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "sanitize-html": "^1.14.1", "text-encoding-utf-8": "^1.0.1", "url": "^0.11.0", + "url-search-params": "^0.10.0", "velocity-vector": "vector-im/velocity#059e3b2", "whatwg-fetch": "^1.0.0" }, From 3756ce606d4809d0dcc9dfe5dc458487cd3a1f8e Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Fri, 27 Oct 2017 17:49:14 +0100 Subject: [PATCH 03/26] Check URL prop updates and ensure that widgets are refreshed. --- src/components/views/elements/AppTile.js | 75 ++++++++++++++---------- 1 file changed, 43 insertions(+), 32 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 7da0407c002..c2f252ce3a8 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -17,6 +17,7 @@ limitations under the License. 'use strict'; import url from 'url'; +import URLSearchParams from 'url-search-params'; import React from 'react'; import MatrixClientPeg from '../../../MatrixClientPeg'; import PlatformPeg from '../../../PlatformPeg'; @@ -51,28 +52,32 @@ export default React.createClass({ creatorUserId: React.PropTypes.string, }, - getDefaultProps: function() { + getDefaultProps() { return { url: "", }; }, - getInitialState: function() { - const widgetPermissionId = [this.props.room.roomId, encodeURIComponent(this.props.url)].join('_'); - const hasPermissionToLoad = localStorage.getItem(widgetPermissionId); - return { - loading: false, - widgetUrl: this.props.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' || this.props.userId === this.props.creatorUserId, - error: null, - deleting: false, - }; + _getInitialState() { + const widgetPermissionId = [this.props.room.roomId, encodeURIComponent(this.props.url)].join('_'); + const hasPermissionToLoad = localStorage.getItem(widgetPermissionId); + return { + loading: true, + widgetUrl: this.props.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' || this.props.userId === this.props.creatorUserId, + error: null, + deleting: false, + }; + }, + + getInitialState() { + return this._getInitialState(); }, // Returns true if props.url is a scalar URL, typically https://scalar.vector.im/api - isScalarUrl: function() { + isScalarUrl() { let scalarUrls = SdkConfig.get().integrations_widgets_urls; if (!scalarUrls || scalarUrls.length == 0) { scalarUrls = [SdkConfig.get().integrations_rest_url]; @@ -86,7 +91,7 @@ export default React.createClass({ return false; }, - isMixedContent: function() { + isMixedContent() { const parentContentProtocol = window.location.protocol; const u = url.parse(this.props.url); const childContentProtocol = u.protocol; @@ -98,17 +103,16 @@ export default React.createClass({ return false; }, - componentWillMount: function() { + componentWillMount() { window.addEventListener('message', this._onMessage, false); this.updateWidgetContent(); }, // Update widget content updateWidgetContent() { + this.setState(this._getInitialState()); + // Set scalar token on the wUrl, if needed this.setScalarToken(); - this.setState({ - loading: true, - }); }, // Adds a scalar token to the widget URL, if required @@ -121,13 +125,13 @@ export default React.createClass({ this._scalarClient = new ScalarAuthClient(); } this._scalarClient.getScalarToken().done((token) => { - // Append scalar_token as a query param + // Append scalar_token as a query param if not already present this._scalarClient.scalarToken = token; const u = url.parse(this.props.url); - if (!u.search) { - u.search = "?scalar_token=" + encodeURIComponent(token); - } else { - u.search += "&scalar_token=" + encodeURIComponent(token); + const params = new URLSearchParams(u.search); + if (!params.get('scalar_token')) { + params.set('scalar_token', encodeURIComponent(token)); + u.search = params.toString(); } this.setState({ @@ -147,8 +151,10 @@ export default React.createClass({ window.removeEventListener('message', this._onMessage); }, - componentWillReceiveProps(nextProps) { - console.warn("Apptile", this.id, "got new props", this.url, nextProps.url); + componentDidUpdate(prevProps) { + if (prevProps.url !== this.props.url) { + this.updateWidgetContent(); + } }, _onMessage(event) { @@ -170,11 +176,11 @@ export default React.createClass({ } }, - _canUserModify: function() { + _canUserModify() { return WidgetUtils.canUserModifyWidgets(this.props.room.roomId); }, - _onEditClick: function(e) { + _onEditClick(e) { console.log("Edit widget ID ", this.props.id); const IntegrationsManager = sdk.getComponent("views.settings.IntegrationsManager"); const src = this._scalarClient.getScalarInterfaceUrlForRoom( @@ -186,7 +192,7 @@ export default React.createClass({ /* If user has permission to modify widgets, delete the widget, otherwise revoke access for the widget to load in the user's browser */ - _onDeleteClick: function() { + _onDeleteClick() { if (this._canUserModify()) { // Show delete confirmation dialog const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); @@ -218,6 +224,10 @@ export default React.createClass({ } }, + _onLoaded() { + this.setState({loading: false}); + }, + // Widget labels to render, depending upon user permissions // These strings are translated at the point that they are inserted in to the DOM, in the render method _deleteWidgetLabel() { @@ -240,7 +250,7 @@ export default React.createClass({ this.setState({hasPermissionToLoad: false}); }, - formatAppTileName: function() { + formatAppTileName() { let appTileName = "No name"; if(this.props.name && this.props.name.trim()) { appTileName = this.props.name.trim(); @@ -248,7 +258,7 @@ export default React.createClass({ return appTileName; }, - onClickMenuBar: function(ev) { + onClickMenuBar(ev) { ev.preventDefault(); // Ignore clicks on menu bar children @@ -263,7 +273,7 @@ export default React.createClass({ }); }, - render: function() { + render() { let appTileBody; // Don't render widget if it is in the process of being deleted @@ -349,6 +359,7 @@ export default React.createClass({ alt={_t('Edit')} title={_t('Edit')} onClick={this._onEditClick} + onLoad={this._onLoaded} /> } { /* Delete widget */ } From 648b295971c8692ccb80d3257fcb511c37b2eddc Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Tue, 31 Oct 2017 10:04:02 +0000 Subject: [PATCH 04/26] Removed comment --- src/components/views/elements/AppTile.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index c2f252ce3a8..eeadabf76af 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -108,7 +108,6 @@ export default React.createClass({ this.updateWidgetContent(); }, - // Update widget content updateWidgetContent() { this.setState(this._getInitialState()); // Set scalar token on the wUrl, if needed From 1cb878bb572ca8fcb9b0233a466be7437e065257 Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Tue, 31 Oct 2017 10:04:37 +0000 Subject: [PATCH 05/26] Formatting --- src/components/views/elements/AppTile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index eeadabf76af..06d223e8e53 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -120,7 +120,7 @@ export default React.createClass({ return; } // Fetch the token before loading the iframe as we need to mangle the URL - if(! this._scalarClient) { + if (!this._scalarClient) { this._scalarClient = new ScalarAuthClient(); } this._scalarClient.getScalarToken().done((token) => { From 355d69b0248fe13784a9d68dec245911ecacd23a Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Tue, 31 Oct 2017 10:15:30 +0000 Subject: [PATCH 06/26] Add querystring dep. already used by things like url_utils.js. --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 9c3dddf346f..0983d6d070a 100644 --- a/package.json +++ b/package.json @@ -74,6 +74,7 @@ "matrix-js-sdk": "0.8.5", "optimist": "^0.6.1", "prop-types": "^15.5.8", + "querystring": "^0.2.0", "react": "^15.4.0", "react-addons-css-transition-group": "15.3.2", "react-dom": "^15.4.0", From 6012b35acf9d2b044bd91cab1820883329561cae Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Tue, 31 Oct 2017 10:22:58 +0000 Subject: [PATCH 07/26] Use querystring lib instead of URLSearchParams --- package-lock.json | 5 ----- package.json | 1 - 2 files changed, 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5810e571f19..6dd02674be2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6241,11 +6241,6 @@ } } }, - "url-search-params": { - "version": "0.10.0", - "resolved": "https://registry.npmjs.org/url-search-params/-/url-search-params-0.10.0.tgz", - "integrity": "sha512-oFPzmbPAbdthStgffGq8alULe47skPDt1X3KW6NOQnKkcLHP4IS1NfdfHG/CBP5lGsr2gDzNp87pfWLx/eIxjw==" - }, "user-home": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/user-home/-/user-home-1.1.1.tgz", diff --git a/package.json b/package.json index 0983d6d070a..9ac765051b9 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,6 @@ "sanitize-html": "^1.14.1", "text-encoding-utf-8": "^1.0.1", "url": "^0.11.0", - "url-search-params": "^0.10.0", "velocity-vector": "vector-im/velocity#059e3b2", "whatwg-fetch": "^1.0.0" }, From 35b33263eaa506f4cca07245add6f87ae83e6435 Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Tue, 31 Oct 2017 10:37:40 +0000 Subject: [PATCH 08/26] Use querystring lib --- src/components/views/elements/AppTile.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 06d223e8e53..cf41aad0bbc 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -17,7 +17,7 @@ limitations under the License. 'use strict'; import url from 'url'; -import URLSearchParams from 'url-search-params'; +import qs from 'querystring'; import React from 'react'; import MatrixClientPeg from '../../../MatrixClientPeg'; import PlatformPeg from '../../../PlatformPeg'; @@ -127,10 +127,10 @@ export default React.createClass({ // Append scalar_token as a query param if not already present this._scalarClient.scalarToken = token; const u = url.parse(this.props.url); - const params = new URLSearchParams(u.search); - if (!params.get('scalar_token')) { - params.set('scalar_token', encodeURIComponent(token)); - u.search = params.toString(); + const params = qs(u.search); + if (!params.scalar_token) { + params.scalar_token = encodeURIComponent(token); + u.search = qs.stringify(params); } this.setState({ From 758df29b239b134bf01263d2dc46134e17b08d8d Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Tue, 31 Oct 2017 10:43:17 +0000 Subject: [PATCH 09/26] Fix onLoad on wrong element. --- src/components/views/elements/AppTile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index cf41aad0bbc..9dc5804aa42 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -317,6 +317,7 @@ export default React.createClass({ src={safeWidgetUrl} allowFullScreen="true" sandbox={sandboxFlags} + onLoad={this._onLoaded} > ); @@ -358,7 +359,6 @@ export default React.createClass({ alt={_t('Edit')} title={_t('Edit')} onClick={this._onEditClick} - onLoad={this._onLoaded} /> } { /* Delete widget */ } From 17c0405862458d4a7f9b9766cee935793d329a18 Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Tue, 31 Oct 2017 16:31:46 +0000 Subject: [PATCH 10/26] Restructure to pass props from componentWillRecieveProps. --- src/components/views/elements/AppTile.js | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 9dc5804aa42..1a6683fc8bd 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -58,22 +58,28 @@ export default React.createClass({ }; }, - _getInitialState() { - const widgetPermissionId = [this.props.room.roomId, encodeURIComponent(this.props.url)].join('_'); + + /** + * Set initial component state when the App wUrl (widget URL) is being updated + * @param {Object} props The component props *must* be passed (rather than using this.props) so that it can be called with future props, e.g. from componentWillReceiveProps. + * @return {Object} Updated component state to be set with setState + */ + _getNewUrlState(props) { + const widgetPermissionId = [props.room.roomId, encodeURIComponent(props.url)].join('_'); const hasPermissionToLoad = localStorage.getItem(widgetPermissionId); return { loading: true, - widgetUrl: this.props.url, + widgetUrl: props.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' || this.props.userId === this.props.creatorUserId, + hasPermissionToLoad: hasPermissionToLoad === 'true' || props.userId === props.creatorUserId, error: null, deleting: false, }; }, getInitialState() { - return this._getInitialState(); + return this._getNewUrlState(this.props); }, // Returns true if props.url is a scalar URL, typically https://scalar.vector.im/api @@ -109,7 +115,7 @@ export default React.createClass({ }, updateWidgetContent() { - this.setState(this._getInitialState()); + this.setState(this._getNewUrlState()); // Set scalar token on the wUrl, if needed this.setScalarToken(); }, @@ -150,9 +156,9 @@ export default React.createClass({ window.removeEventListener('message', this._onMessage); }, - componentDidUpdate(prevProps) { - if (prevProps.url !== this.props.url) { - this.updateWidgetContent(); + componentWillReceiveProps(nextProps) { + if (nextProps.url !== this.props.url) { + this.updateWidgetContent(nextProps); } }, From a52bb9d6039d52a255b160a2da007b06b6cfe48a Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Thu, 2 Nov 2017 17:27:59 +0000 Subject: [PATCH 11/26] Pass URL to check. --- src/components/views/elements/AppTile.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 1a6683fc8bd..a440c1703ce 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -82,15 +82,23 @@ export default React.createClass({ return this._getNewUrlState(this.props); }, - // Returns true if props.url is a scalar URL, typically https://scalar.vector.im/api - isScalarUrl() { + /** + * Returns true if specified url is a scalar URL, typically https://scalar.vector.im/api + * @param {[type]} url URL to check + * @return {Boolean} True if specified URL is a scalar URL + */ + isScalarUrl(url) { + if (!url) { + return false; + } + let scalarUrls = SdkConfig.get().integrations_widgets_urls; if (!scalarUrls || scalarUrls.length == 0) { scalarUrls = [SdkConfig.get().integrations_rest_url]; } for (let i = 0; i < scalarUrls.length; i++) { - if (this.props.url.startsWith(scalarUrls[i])) { + if (url.startsWith(scalarUrls[i])) { return true; } } @@ -122,7 +130,7 @@ export default React.createClass({ // Adds a scalar token to the widget URL, if required setScalarToken() { - if (!this.isScalarUrl()) { + if (!this.isScalarUrl(this.props.url)) { return; } // Fetch the token before loading the iframe as we need to mangle the URL From 0e854ee356405f1a3598b4b9592b57273545f83f Mon Sep 17 00:00:00 2001 From: Richard Lewis Date: Thu, 2 Nov 2017 18:33:11 +0000 Subject: [PATCH 12/26] Fix loading and initialisation spinners. --- src/components/views/elements/AppTile.js | 72 +++++++++++++----------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index a440c1703ce..1054a6c4654 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -58,28 +58,28 @@ export default React.createClass({ }; }, - /** * Set initial component state when the App wUrl (widget URL) is being updated * @param {Object} props The component props *must* be passed (rather than using this.props) so that it can be called with future props, e.g. from componentWillReceiveProps. * @return {Object} Updated component state to be set with setState */ _getNewUrlState(props) { - const widgetPermissionId = [props.room.roomId, encodeURIComponent(props.url)].join('_'); - const hasPermissionToLoad = localStorage.getItem(widgetPermissionId); - return { - loading: true, - widgetUrl: props.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' || props.userId === props.creatorUserId, - error: null, - deleting: false, - }; + const widgetPermissionId = [props.room.roomId, encodeURIComponent(props.url)].join('_'); + const hasPermissionToLoad = localStorage.getItem(widgetPermissionId); + return { + initialising: true, // True while we are mangling the widget URL + loading: true, // True while the iframe content is loading + widgetUrl: props.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' || props.userId === props.creatorUserId, + error: null, + deleting: false, + }; }, getInitialState() { - return this._getNewUrlState(this.props); + return this._getNewUrlState(this.props); }, /** @@ -119,21 +119,24 @@ export default React.createClass({ componentWillMount() { window.addEventListener('message', this._onMessage, false); - this.updateWidgetContent(); - }, - - updateWidgetContent() { - this.setState(this._getNewUrlState()); - // Set scalar token on the wUrl, if needed this.setScalarToken(); }, - // Adds a scalar token to the widget URL, if required + /** + * Adds a scalar token to the widget URL, if required + * Component initialisation is only complete when this function has resolved + */ setScalarToken() { if (!this.isScalarUrl(this.props.url)) { + this.setState({ + error: null, + widgetUrl: this.props.url, + initialising: false, + }); return; } - // Fetch the token before loading the iframe as we need to mangle the URL + + // Fetch the token before loading the iframe as we need it to mangle the URL if (!this._scalarClient) { this._scalarClient = new ScalarAuthClient(); } @@ -150,12 +153,12 @@ export default React.createClass({ this.setState({ error: null, widgetUrl: u.format(), - loading: false, + initialising: false, }); }, (err) => { this.setState({ error: err.message, - loading: false, + initialising: false, }); }); }, @@ -166,7 +169,8 @@ export default React.createClass({ componentWillReceiveProps(nextProps) { if (nextProps.url !== this.props.url) { - this.updateWidgetContent(nextProps); + this._getNewUrlState(nextProps); + this.setScalarToken(); } }, @@ -308,24 +312,26 @@ export default React.createClass({ } if (this.props.show) { - if (this.state.loading) { + if (this.state.initialising) { appTileBody = (
); } else if (this.state.hasPermissionToLoad == true) { + const loadingElement = ( +
+ +
+ ); if (this.isMixedContent()) { - appTileBody = ( -
- -
- ); + appTileBody = loadingElement; } else { appTileBody = ( -
+
+ { this.loading && Element }