Skip to content

Conversation

@danxuliu
Copy link
Member

@danxuliu danxuliu commented Oct 2, 2020

This pull request adds avatars for guests in the call view, as well as fixing other related issues. For details please refer to the individual commit messages :-)

It will probably need to be forwardported, although I have not checked.

Also note that the avatar is currently shown only for guests for which there is a peer connection (as the guest avatar depends on the nick, and the nick for guests when the HPB is used is currently sent through data channels, which require a peer connection). None of the changes from nextcloud/spreed#4182 are applied here.

How to test (scenario 1)

  • Setup the HPB
  • Create a public conversation
  • With the Android app, join the public conversation
  • With a browser, join the public conversation as a guest

Result with this pull request

Nothing strange happens.

Result without this pull request

The web socket of the Android app gets reconnected.

How to test (scenario 2)

  • Setup the HPB
  • Create a public conversation
  • With a browser, join the public conversation as a guest and start a call
  • With the Android app, join the public conversation and join the call
  • In the browser, change the guest name

Result with this pull request

The guest name is updated.

Result without this pull request

The guest name is not updated. Also, checking the errors in Android's logcat it can be seen that NullPointerExceptions are thrown when the guest sends a nickChanged event (which happens every second).

How to test (scenario 3)

  • This change applies both when the HPB is used and when it is not used. Important: I have tested it only with the HPB; it should work without HPB too, but testing is very much welcome :-)
  • Create a public conversation
  • With a browser, join the public conversation as a guest and start a call
  • With the Android app, join the public conversation and join the call
  • In the browser, change the guest name

Result with this pull request

The guest avatar is initially shown and then updated when the guest name is changed.

Result without this pull request

The guest avatar is not shown.

When the standalone signaling server is used and a participant joins a
conversation a "join" signaling message is received. The code assumed
that the event always contains a "user" attribute, but that attribute is
empty for guests. As the "user" attribute was used unconditionally this
caused an exception, and as the exception happened when handling a
websocket message it propagated until it caused a reconnection of the
websocket.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The "nick" attribute was set with itself instead of with the
"internalNick" variable that holds the value received in the
"nickChanged" event.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Before the NICK_CHANGE event include either the session id or the user
id, depending on whether the participant was a guest or a user. However,
as the session id is also known for users the event can be unified to
always include the session id only.

This also fixes an exception when handling the "NICK_CHANGE" event, as
the session id was got from the user id given in the event, but if the
event already included the session id the look up failed and the session
id was replaced with an empty value. This in turn caused an exception
when trying to use the view for the now invalid session id. Now the
session id provided in the event is always directly used.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The avatar of guests is based on their display name/nick.

When the HPB is not used the nick is provided in the offer/answer
signaling messages, and later updated through data channel messages when
it changes. When the HPB is used the nick is periodically sent through
data channel messages. Therefore the avatar is based on the nick set in
the peer connection and reloaded when the nick changes (although it is
currently a bit hacky and brittle, as it is based on whether the nick
shown in the text view changed rather than whether the nick itself
changed, but it works nevertheless).

Note that currently it is required that the guest has a peer connection
to know its nick and, therefore, its avatar; some changes would be
needed in the clients to also send the nick when there is no peer
connection.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu danxuliu added bug Something isn't working 3. to review Waiting for reviews labels Oct 2, 2020
@dasbiswajit dasbiswajit self-requested a review October 2, 2020 22:30
Copy link

@dasbiswajit dasbiswajit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect tested all scenarios.

@dasbiswajit dasbiswajit merged commit 4f83d47 into v.8.0.x Oct 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-some-issues-related-to-guest-participants branch October 2, 2020 22:50
@tobiasKaminsky tobiasKaminsky added the portToBrokenMaster All PRs to master should also go to master-broken for now label Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug Something isn't working portToBrokenMaster All PRs to master should also go to master-broken for now

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants