fix(prosody-auth): Don't loose initial tracks. #14358
Conversation
hristoterezov
force-pushed
the
fix-prosody-auth-local-tracks
branch
from
07e6513
to
27ab4e5
Compare
During the prosody login cycle the initial GUM tracks were lost causing the user to start the call without local media and audio/video mute buttons staying forever in pending state.
hristoterezov
force-pushed
the
fix-prosody-auth-local-tracks
branch
from
f9a9ee6
to
ec46340
Compare
|
|
||
| return APP.store.dispatch(initPrejoin(localTracks, errors)); | ||
| // Note: Not sure if initPrejoin needs to be async. But let's wait for it just to be sure the |
There was a problem hiding this comment.
It doesn't need to be. It doesn't even need access to the store so it doesn't even need to be async.
export function initPrejoin(tracks: Object[], errors: Object) {
return async function(dispatch: IStore['dispatch']) {
dispatch(setPrejoinDeviceErrors(errors));
dispatch(prejoinInitialized());
tracks.forEach(track => dispatch(trackAdded(track)));
};
}
You could rewrite this to take the store as a parameter and just call it, since it's not really an action. Then in there you can batch-dispatch the actual actions.
| // dispatch(connect(jid, password)); | ||
| // FIXME: Workaround for the web version. To be removed once we get rid of conference.js | ||
| dispatch(joinConference(undefined, false, jid, password)); | ||
| dispatch(connect(jid, password)); |
There was a problem hiding this comment.
Has this been tested with XMPP auth? I recall we've broken it a number of times in the past.
| @@ -153,6 +158,11 @@ MiddlewareRegistry.register(store => next => action => { | |||
| error.recoverable = true; | |||
|
|
|||
| _handleLogin(store); | |||
| } else { | |||
| batch(() => { | |||
There was a problem hiding this comment.
This feeels like the wrong place to be handling this. Authentication should not be handling track stuff.
You can handle this in the media middleware, since the recoverable flag will already be set once you calll next when handling this action.
| @@ -264,6 +274,11 @@ function _handleLogin({ dispatch, getState }: IStore) { | |||
| const videoMuted = isLocalTrackMuted(state['features/base/tracks'], MEDIA_TYPE.VIDEO); | |||
|
|
|||
| if (!room) { | |||
| batch(() => { | |||
| APP.conference.startConference(jitsiTracks).catch(logger.error); | ||
| } | ||
| }); | ||
| .then(() => dispatch(disconnect())); |
There was a problem hiding this comment.
When will the connection begin again?
There was a problem hiding this comment.
I see some of the logic got moved to base/conference, but only to the web middleware. When does mobile start connecting again?
| * @param {string} action.type - Type of action. | ||
| * @returns {ICommonState} | ||
| */ | ||
| function _common(state: ICommonState = _COMMON_INITIAL_STATE, action: AnyAction) { |
There was a problem hiding this comment.
Generally speaking, base/media has always represented user intent, that is, a muted a flag in here does not necesssarily mean there is a track.
The initial gUM promise is concerned with actual tracks, which fits better in base/tracks.
During the prosody login cycle the initial GUM tracks were lost causing the user to start the call without local media and audio/video mute buttons staying forever in pending state.