Monal icon indicating copy to clipboard operation
Monal copied to clipboard

[FEATURE]: implement like lissine suggested in last comment

Open lissine0 opened this issue 1 year ago • 21 comments

What Monal Release channel are you using?

Alpha

iOS system version

16.7

iOS Monal version

6.0 (commit ef03f7b)

macOS system version

No response

macOS Monal version

No response

Used XMPP server (domain)

personal server

Which XMPP-Server software are you using?

Prosody

XMPP Server Software Version

0.12.4

How many accounts are you using in Monal?

2

What happened?

If Monal isn't displaying the Chat view for a MUC, and another client deletes its bookmark or removes the autojoin flag, then Monal considers the MUC a one-to-one chat. And It deletes the avatar and the previous history. You may need to close the app and re-open it to take effect. Steps to reproduce:

  1. Join a channel using Monal
  2. Make sure that the channel's chat view isn't displayed by Monal. (for example go to another view, or close the device)
  3. Delete the bookmark using Conversations, or leave the channel using Gajim
  4. If Monal is open, close it and re-open it.
  5. The channel's avatar and history is gone. And if you enter it, it tells you that it couldn't find omemo keys. And the call icon is visible

Anything else?

If instead of step 2. you open the channel's chat view, then when the bookmark/autojoin is deleted, Monal will go back to the Chats view, and the channel in question will be nowhere to be seen (neither in the Chats list, nor in the Contacts list).

FAQ

Considerations for XMPP users

Related Issues

  • [X] I have cross-checked this overview https://github.com/monal-im/Monal/issues/322 as well as filtered for related labels https://github.com/monal-im/Monal/labels

XEP-Check

  • [X] I have checked that at least XEP-198, XEP-0280, XEP-0352, XEP-0357, XEP-0313 and XEP-0163 are activated on my server and shown as 'green' under Settings --> Account--> (i) in advanced settings

Notifications-Menu

  • [X] I have checked that all checkmarks are present under Settings --> Notifications

lissine0 avatar Oct 08 '23 23:10 lissine0

I need a logfile to debug this

tmolitor-stud-tu avatar Oct 13 '23 02:10 tmolitor-stud-tu

My initial description was a bit inaccurate. Actual steps to reproduce:

  1. Join a channel using Monal
  2. Make sure that the channel's chat view isn't displayed by Monal, by going to another view (Chats for example)
  3. Delete the bookmark using Conversations/Dino, or leave the channel using Gajim
  4. Wait a couple of seconds for Monal to get the update in bookmarks, then display the deleted channel's chat view
  5. Close Monal and re-open it.
  6. The channel's avatar and history is gone. And if you enter it, it tells you that it couldn't find omemo keys. And the call icon is visible

Explanation: After you leave the channel with the other client, the conversation isn't removed immediately from Chats. (but it's removed from Contacts page if you close that page and reopen it). I think it's cached. And you need to close the app and reopen it for the conversation to be gone.

The cause of the bug is clicking on the conversation after its bookmark's deletion (but before closing and reopening the app). Presumably, Monal gets confused since it can't find the relevant bookmark. So it just assumes it's a 1-1 chat.

lissine0 avatar Oct 13 '23 19:10 lissine0

is this still happening in the latest beta?

tmolitor-stud-tu avatar Nov 09 '23 17:11 tmolitor-stud-tu

Still reproducible on 6.0.1 (870)

lissine0 avatar Nov 09 '23 18:11 lissine0

Just ran into such a situation with the latest alpha. Re-adding and then removing contact resolved the situation, however certainly would be nice to avoid such a situation in the first place.

foss- avatar Nov 10 '23 12:11 foss-

Can you check if this is fixed now using latest alpha or beta builds?

tmolitor-stud-tu avatar Feb 24 '24 20:02 tmolitor-stud-tu

Yes, this is indeed fixed on the latest beta. Thanks! However, there's some inconsistency between leaving a channel on Monal vs leaving it on another device:

  • If I leave it on Monal, the channel disappears from the "Chats" view
  • If I leave if from another device, the channel remains on the "Chats" view despite being un-joined (I can read its local history, but I cannot fetch its history or new messages from the server)

It would be nice to have the same treatment in both cases Thanks!!

lissine0 avatar Feb 26 '24 20:02 lissine0

great! how does Conversations behave if the muc is removed from bookmarks by another client? I'd like to match those behaviours.

tmolitor-stud-tu avatar Feb 26 '24 20:02 tmolitor-stud-tu

@tmolitor-stud-tu If I close a MUC in Gajim, which removes the autojoin flag, Conversations also closes the MUC. If I join a MUC in Gajim, Conversations opens the MUC (but sometimes does not join it, if the MUC publishes your XMPP address). If I close a MUC in Conversations, which sets autojoin to "false", Gajim does nothing.

haansn08 avatar Feb 26 '24 21:02 haansn08

Okay, I think not joining the muc is moot because the client adding that bookmark is most likely already joined as well.

Other than that, I think I'll change Monal to match the behavior of Conversations

And I consider Gajim not leaving the muc a bug.

tmolitor-stud-tu avatar Feb 26 '24 21:02 tmolitor-stud-tu

Currently Monal does the following: keep the entry in the "Chats" view if only the autojoin flag was removed and remove the entry if the whole bookmark was removed.

Would that be sensible, too? Are you able to remove the whole bookmark using Conversations? Or is conversations always only removing the autojoin flag?

tmolitor-stud-tu avatar Feb 26 '24 21:02 tmolitor-stud-tu

According to XEP-0402: PEP Native Bookmarks:

if the event is a retract notification, the client SHOULD leave the room immediately.

Conversations doesn't follow this "SHOULD". It stays joined to the room.

In my opinion, we shouldn't have a state where we're not joined to the channel but it is in the main "Chats" view. Because that would be confusing, as the user can't easily differentiate between joined channels and un-joined ones; unless some UI is added, like greying out the channel's avatar.

This is the case in Conversations as well: having a channel in the main "Chats" view means you're joined to that channel (unless the internet if off)

@haansn08 Gajim isn't fully compatible with XEP-402 yet. See
https://dev.gajim.org/gajim/gajim/-/issues/11288

lissine0 avatar Feb 26 '24 21:02 lissine0

Currently Monal does the following: keep the entry in the "Chats" view if only the autojoin flag was removed and remove the entry if the whole bookmark was removed.

Conversations removes the entry from the Chats window if the autojoin flag becomes unset. If you want to re-join you need to "Start Conversation" -> "Group Chats" tab -> Click on a bookmarked MUC.

Would that be sensible, too? Are you able to remove the whole bookmark using Conversations? Or is conversations always only removing the autojoin flag?

Closing the MUC in the Chats view sets autojoin to false, but does NOT remove the bookmark. It is possible to remove the bookmark entirely by going to "Group chat details" -> "Delete bookmark" or "Start Conversation" -> "Group Chats" -> Long press on any bookmarked MUC -> "Delete bookmark".

haansn08 avatar Feb 26 '24 21:02 haansn08

Currently Monal does the following: keep the entry in the "Chats" view if only the autojoin flag was removed and remove the entry if the whole bookmark was removed.

Would that be sensible, too? Are you able to remove the whole bookmark using Conversations? Or is conversations always only removing the autojoin flag?

Conversations and Gajim allow you to do both of those things. The idea behind having a bookmark with autojoin=false, is that it may be useful in the future (like a browser bookmark) bookmarks_screenshot Generally in other clients, setting autojoin=false is coupled with leaving the channel AFAIK Monal already behaves correctly to what other clients do wrt joining / leaving channels. This issue (https://github.com/monal-im/Monal/issues/955) is about a bug in the app's UI logic, not the protocol logic. Same goes to my first comment from today that started the conversation Also, I'm sorry but I just found out the issue isn't actually fixed. I initially tested using the steps in OP but they're inaccurate as mentioned in https://github.com/monal-im/Monal/issues/955#issuecomment-1762069754 In fact, you don't need other clients to trigger this bug.

Steps to reproduce it simply:

1- Go to the "Contacts" view 2- Go to the details view of the concerned channel 3- Leave Channel 4- Close the "Contacts" view 5- Enter the chat of the channel you just left, you'll find it empty of chat history 6- Double tap the home button, close Monal and re-open it 7- now even the avatar and the name of the channel are gone (name replaced by jid localpart) and Monal thinks it's a 1-1 chat

The issue is triggered by step 5, and you shouldn't have been allowed to do step 5- after you did step 3- This can be solved by triggering / queuing a refresh of some sort after leaving the channel. A simple workaround is closing Monal and reopening it after leaving a channel, and before doing more navigation around the app

Back to the protocol discussion, if you agreed with this idea

The idea behind having a bookmark with autojoin=false, is that it may be useful in the future (like a browser bookmark)

Then you can add a special UI for this kind of bookmarks in the "Contacts" view. For example, these bookmarks would be at the bottom with the corresponding avatars greyed-out, as Movim does it. Then the process would be: you leave a channel, it gets removed from the main "Chats" view but its bookmark is still in the "Contacts" view, albeit with a greyed-out avatar and at the very bottom. You enter its details from there and you click "Forget about this channel" or something like that and it's gone from your bookmarks.

lissine0 avatar Feb 27 '24 00:02 lissine0

This issue (https://github.com/monal-im/Monal/issues/955) is about a bug in the app's UI logic, not the protocol logic.

Yes, I know. I just wanted to know your opinion on the protocol logic.

For the UI rewrite we are planning to add a sticky notification into the chat that tells the user that they are not joined to that channel/group (with a button to quickly join). I'm not sure if that's a better approach than leaving the muc listed (greyed out) in the contact list and removing it from the "active chats" view. What do you think?

As for the original issue: yes, you are right, there is just a refresh missing. But I thought I added just such a refresh in some of my recent commits, hence my question if the bug still persists. Sad to hear that this was apparently still not enough.

tmolitor-stud-tu avatar Feb 27 '24 03:02 tmolitor-stud-tu

I believe my latest alpha commit solves the issue, can you confirm that?

tmolitor-stud-tu avatar Feb 27 '24 04:02 tmolitor-stud-tu

@lissine0 ping (want to confirm this before releasing the next beta)

tmolitor-stud-tu avatar Feb 28 '24 21:02 tmolitor-stud-tu

Yes it is indeed fixed. (Sorry I forgot to reply)

lissine0 avatar Feb 28 '24 21:02 lissine0

For the UI rewrite we are planning to add a sticky notification into the chat that tells the user that they are not joined to that channel/group (with a button to quickly join). I'm not sure if that's a better approach than leaving the muc listed (greyed out) in the contact list and removing it from the "active chats" view. What do you think

The latter approach is better IMHO.

If the user isn't joined to a channel / group they wouldn't get any new messages. So I think it's better to remove unjoined MUCs from "active chats" because they wouldn't be active. Besides, having bookmarks with autojoin=flase in the "Contacts" page would be similar behavior to other clients like Conversations, Gajim and Dino, which show [all kinds of] bookmarks and Contacts in the same place. Moreover, when you want to start a new chat, you usually go through the Contacts page. And finally, it would be easier if all the unjoined channels / groups are together in one section. (if they're to be in the main view, they'd be scattered through the list depending on the last message time)

lissine0 avatar Feb 28 '24 21:02 lissine0

Thanks for your insights, that are indeed good arguments :)

Great that it is fixed now!

tmolitor-stud-tu avatar Feb 28 '24 22:02 tmolitor-stud-tu

TODO: implement like lissine suggested

tmolitor-stud-tu avatar Mar 24 '24 20:03 tmolitor-stud-tu