Chat state manager issue

[bangarharshit]

Many servers like ejabberd allows messages with empty body to be stored offline. XEP-160 is not correctly implemented since they are also storing chatstates - processone/ejabberd#842

With customchatlistener there is no way to remove states which are from offline stores since message is not passed to onstatechanged. So in chatstatemanager removing any chatstate which have delay element.

[Flowdalic]

Thanks for your contribution. I'm unsure about this change, but tend to include it. While I make up my mind, could you update your PR so that the commit includes are more meaningful commit message? Describe what you have changed and why. See also https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors#providing-patches-

[xnyhps]

Copying my comments from jdev so they don't get lost:

There is no easy way to distinguish offline messages from messages that have gotten delayed for a different reason, the presence of <delay/> is not enough. For example, a XEP-0198 resume somewhere along the line often also leads to messages being stamped with <delay/> too. There might even be some clients out there that add a <delay/> to every outgoing message.

XEP-0160 is only "Informational", there might be good reasons to store chat states, for example to determine whether you message has been seen by your contact.

[bangarharshit]

Actually for something like composing state I would like to drop it when XEP-0198 resume is there. It should be real time. For something like paused/active state it can be useful but again keep showing istyping on client for that long time is not great. I myself taking care of it by using a delayed runnable to update the state in case no done/paused state arrives.

Again using active and inactive state to determine seen is nice and we need delayed element to determine which all messages are seen (I used a separate namespace with message to determine that). But getting ghost message like A is composing even after A is offline for last 5 hours is very bad experience. With current chat state listener there is no way to filter out message since only chat and chatstate. One workaround is to not use chatstate listener and write your custom logic inside message listener with check for that namespace.

Do you think if we can pass delayedtime to chatstatechanged method. The client will then accordingly take the action.

[Flowdalic]

Do you think if we can pass delayedtime to chatstatechanged method.

I was thinking along these lines as solution. But typically you would not only pass a DelayedInformation reference down the user, but simply the whole message stanza.

So

stateChanged(Chat, ChatState)

becomes

stateChanged(Chat, ChatState, Message)

@bangarharshit Would you volunteer to write the patch for this? :)

[damencho]

Just saw this one :) and wanted to give 👍 for it, as we are already using this in our patched smack 3.2 version :). (stateChanged(Chat, ChatState, Message))
jitsi/smack_3_2_2@f11ec89

[Flowdalic]

Thanks @damencho

Tracking this as SMACK-704.

[bangarharshit]

Hey Flow,
Thanks for the feedback. Stuck with some delayed stanza acknowledgement issue. I can update the patch by tomorrow.

[Flowdalic]

Great, please make sure to include the issue key SMACK-704 somewhere in the commit message, e.g. "Fixes SMACK-704".

[bangarharshit]

Sure 👍

[Flowdalic]

ping

[Flowdalic]

Fixed with 0761fe0