From e403169e135a207f921ad48b5b36d20aa04f9ea2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 29 Jan 2020 16:56:12 +0000 Subject: [PATCH 1/3] Fix various races that prevented the right panel being in the right state for verifications Fixes https://github.com/vector-im/riot-web/issues/11989 --- src/components/structures/RightPanel.js | 22 ++++++++++++++++++- .../views/right_panel/EncryptionPanel.js | 2 +- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index be10ead7ca..8dd44e37d4 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -48,6 +48,7 @@ export default class RightPanel extends React.Component { phase: this._getPhaseFromProps(), isUserPrivilegedInGroup: null, member: this._getUserForPanel(), + verificationRequest: RightPanelStore.getSharedInstance().roomPanelPhaseParams.verificationRequest, }; this.onAction = this.onAction.bind(this); this.onRoomStateMember = this.onRoomStateMember.bind(this); @@ -68,15 +69,34 @@ export default class RightPanel extends React.Component { return this.props.user || lastParams['member']; } + // gets the current phase from the props and also maybe the store _getPhaseFromProps() { const rps = RightPanelStore.getSharedInstance(); + const userForPanel = this._getUserForPanel(); if (this.props.groupId) { if (!RIGHT_PANEL_PHASES_NO_ARGS.includes(rps.groupPanelPhase)) { dis.dispatch({action: "set_right_panel_phase", phase: RIGHT_PANEL_PHASES.GroupMemberList}); return RIGHT_PANEL_PHASES.GroupMemberList; } return rps.groupPanelPhase; - } else if (this._getUserForPanel()) { + } else if (userForPanel) { + // XXX FIXME AAAAAARGH: What is going on with this class!? It takes some of its state + // from its props and some from a store, expect if the contents of the store changes + // while it's mounted in which case it replaces all of its state with that of the store, + // expect it uses a dispatch instead of a normal store listener? + // Unfortunately rewriting this would almost certainly break showing the right panel + // in some of the many cases, and I don't have time to re-architect it and test all + // the flows now, so adding yet another special case so if the store thinks there is + // a verification going on for the member we're displaying, we show that, otherwise + // we race if a verification is started while the panel isn't displayed because we're + // not mounted in time to get the dispatch. + // Until then, let this code serve as a warning from history. + if ( + userForPanel.userId === rps.roomPanelPhaseParams.member.userId && + rps.roomPanelPhaseParams.verificationRequest + ) { + return rps.roomPanelPhase; + } return RIGHT_PANEL_PHASES.RoomMemberInfo; } else { if (!RIGHT_PANEL_PHASES_NO_ARGS.includes(rps.roomPanelPhase)) { diff --git a/src/components/views/right_panel/EncryptionPanel.js b/src/components/views/right_panel/EncryptionPanel.js index 6819db8e55..f25a55fa8a 100644 --- a/src/components/views/right_panel/EncryptionPanel.js +++ b/src/components/views/right_panel/EncryptionPanel.js @@ -36,7 +36,7 @@ const EncryptionPanel = ({verificationRequest, member, onClose}) => { setRequest(verificationRequest); }, [verificationRequest]); - const [phase, setPhase] = useState(undefined); + const [phase, setPhase] = useState(request.phase); const changeHandler = useCallback(() => { // handle transitions -> cancelled for mismatches which fire a modal instead of showing a card if (request && request.cancelled && MISMATCHES.includes(request.cancellationCode)) { From de6ef9ec90536ac6e042e17502c3e3e174be85a1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 29 Jan 2020 17:51:51 +0000 Subject: [PATCH 2/3] words Co-Authored-By: J. Ryan Stinnett --- src/components/structures/RightPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index 8dd44e37d4..bf4e99ee51 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -81,7 +81,7 @@ export default class RightPanel extends React.Component { return rps.groupPanelPhase; } else if (userForPanel) { // XXX FIXME AAAAAARGH: What is going on with this class!? It takes some of its state - // from its props and some from a store, expect if the contents of the store changes + // from its props and some from a store, except if the contents of the store changes // while it's mounted in which case it replaces all of its state with that of the store, // expect it uses a dispatch instead of a normal store listener? // Unfortunately rewriting this would almost certainly break showing the right panel From d63bb240aa45eaef28396f609eef7cb05dc1f98e Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 29 Jan 2020 17:52:12 +0000 Subject: [PATCH 3/3] those words again Co-Authored-By: J. Ryan Stinnett --- src/components/structures/RightPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index bf4e99ee51..45722caa98 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -83,7 +83,7 @@ export default class RightPanel extends React.Component { // XXX FIXME AAAAAARGH: What is going on with this class!? It takes some of its state // from its props and some from a store, except if the contents of the store changes // while it's mounted in which case it replaces all of its state with that of the store, - // expect it uses a dispatch instead of a normal store listener? + // except it uses a dispatch instead of a normal store listener? // Unfortunately rewriting this would almost certainly break showing the right panel // in some of the many cases, and I don't have time to re-architect it and test all // the flows now, so adding yet another special case so if the store thinks there is