From 8dc21ec83721bf8b7b9b8289d9d9e1867e28e5fd Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 20 Jan 2016 15:25:40 +0000 Subject: [PATCH 1/2] Fix race condition when joining rooms Problem: Hitting join on a room invite would show spinner, then join room screen then the messages. Cause: The UI to show a spinner is set via the "joining" flag. This flag was only set for the duration of the /join HTTP request. This is insufficient because it races with actual room info arriving from /sync. If this info does not arrive before the /join HTTP request returns, "joining" is false but the current membership state of the user is still invite. Fix: The "joining" flag is still set when the /join HTTP request is made, but it is only turned off when the join event arrives from /sync. Extras: This fix should also work when joining a room not from an invite. --- src/components/structures/RoomView.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 3cf3f4aafc..951a9a5ec1 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -379,6 +379,14 @@ module.exports = React.createClass({ if (member.roomId === this.props.roomId) { // a member state changed in this room, refresh the tab complete list this._updateTabCompleteList(this.state.room); + + var room = MatrixClientPeg.get().getRoom(this.props.roomId); + var me = MatrixClientPeg.get().credentials.userId; + if (this.state.joining && room.hasMembershipState(me, "join")) { + this.setState({ + joining: false + }); + } } if (!this.props.ConferenceHandler) { @@ -552,10 +560,17 @@ module.exports = React.createClass({ onJoinButtonClicked: function(ev) { var self = this; - MatrixClientPeg.get().joinRoom(this.props.roomId).then(function() { + MatrixClientPeg.get().joinRoom(this.props.roomId).done(function() { + // It is possible that there is no Room yet if state hasn't come down + // from /sync - joinRoom will resolve when the HTTP request to join succeeds, + // NOT when it comes down /sync. If there is no room, we'll keep the + // joining flag set until we see it. Likewise, if our state is not + // "join" we'll keep this flag set until it comes down /sync. + var room = MatrixClientPeg.get().getRoom(self.props.roomId); + var me = MatrixClientPeg.get().credentials.userId; self.setState({ - joining: false, - room: MatrixClientPeg.get().getRoom(self.props.roomId) + joining: room ? !room.hasMembershipState(me, "join") : true, + room: room }); }, function(error) { self.setState({ From b296d05b350ad4d4a915e983e2a95630d4e12d79 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 20 Jan 2016 15:38:34 +0000 Subject: [PATCH 2/2] Fix finally NPE --- src/components/structures/RoomView.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 951a9a5ec1..e2a9f8b730 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -118,15 +118,11 @@ module.exports = React.createClass({ if (!this.state.room) { if (this.props.autoPeek) { console.log("Attempting to peek into room %s", this.props.roomId); - MatrixClientPeg.get().peekInRoom(this.props.roomId).done(() => { - // we don't need to do anything - JS SDK will emit Room events - // which will update the UI. We *do* however need to know if we - // can join the room so we can fiddle with the UI appropriately. - - // ...XXX: or do we? can't we just do them onNewRoom? - }, function(err) { + MatrixClientPeg.get().peekInRoom(this.props.roomId).catch((err) => { console.error("Failed to peek into room: %s", err); }).finally(() => { + // we don't need to do anything - JS SDK will emit Room events + // which will update the UI. this.setState({ autoPeekDone: true });