Conversation
|
This does not look like a fixable bug because you don't know if the message was incorrectly timestamped into the future or the message is timestamped correctly and the clock is now incorrectly set to the past. This is also not what was reported in #8027. Inventing theoretical cases and fixing them one by one will just result in more complicated logic and more complicated bugs, we tried this with memberlist management and eventually had to throw away all the old logic and redesign it from scratch. |
Strictly speaking, yes, but for incoming messages this is actually fixed by
In this issue the user set their clock to the past by mistake and was experiencing sent messages added to the middle of the chat. It's not much different from setting the clock to the future by mistake and then fixing it back. |
|
If there is a reason why we sort received messages to the bottom (even if the user set their clock to the past), but we shouldn't do this for sent messages, this PR should be closed, but the reason should be documented as a comment at least. |
link2xt
left a comment
There was a problem hiding this comment.
Let's merge this, it is a bit unfortunate that this adds a rather complicated SQL query but likely fixes the case of active chatting between senders that have clocks desynced by ~10 seconds or so which is somewhat expected (#8088 (comment)).
| UPDATE msgs SET | ||
| timestamp=( | ||
| SELECT MAX(timestamp) FROM msgs WHERE | ||
| state IN(10,13,16,18,20,24,26,28) AND |
There was a problem hiding this comment.
Could you add comments like
10, -- InFresh
13, -- InNoticed
...
28, -- OutMdnRcvd
and say that OutDraft is excluded? It is not really possible to see what is happening here without cross-referencing to MessageState and checking what each number means.
There was a problem hiding this comment.
Added a shorter comment. I think it should be enough, the only reason why message states are explicitly enumerated is making the index work
I couldn't understand how this fixes the referenced problem. The comment says:
But this PR only adjusts sent messages to the bottom. This PR can fix things if a sent message is rendered slowly e.g. because it's big, so before this PR it could appear in the chat above messages just received. |
Before, if the user fixed their clock incorrectly set to the future, they needed to delete previously sent messages or wait until this future comes again so that new sent messages are added to the bottom. Strictly speaking, the problem isn't fixable because we don't know if messages were incorrectly timestamped into the future or they are timestamped correctly and the clock is now incorrectly set to the past. Anyway, adding messages to the middle of the chat isn't a good way to inform the user about the problem.
d6f8e30 to
db7b7be
Compare
I agree, I thought about the problem the other way round, where your message is sent and appears above, but this cannot actually happen just during active chatting if your clock does not jump back because all already received messages must be in the past as we normally cap the timestamps to not be in the future. |
Before, if the user fixed their clock incorrectly set to the future, they needed to delete
previously sent messages or wait until this future comes again so that new sent messages are added
to the bottom. Strictly speaking, the problem isn't fixable because we don't know if messages were
incorrectly timestamped into the future or they are timestamped correctly and the clock is now
incorrectly set to the past. Anyway, adding messages to the middle of the chat isn't a good way to
inform the user about the problem.
Fix #8027