-
-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: onEventMessagesChanged
: handle msgId == 0
#4555
Conversation
acaa631
to
fe6b61c
Compare
fe6b61c
to
79be6eb
Compare
What is DC_MSG_ID_LAST_SPECIAL? Is it documented somewhere? And what kind of bug does this fix? |
79be6eb
to
adab9c9
Compare
We have "special" message IDs and "normal" message IDs. All special message IDs are <= than DC_MSG_ID_LAST_SPECIAL.
Should be in core, but I haven't seen the docs specifically for this. |
adab9c9
to
fd1a83f
Compare
fd1a83f
to
432bff1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes the reported problem!
No need for apologize - regarding the number of contributions bugs can happen :-)
This sometimes causes the message list to be empty when the chat gets opened, and not update on some events. Reproduction steps are roughly as follows: 1. Create two accounts 2. On Alice, send ~20 messages to Bob. 3. Switch to Bob's account and open the chat with Alice. Do not scroll to bottom. 4. Switch to Alice account and send another message to Bob. 5. Switch to Bob's account and open the chat with Alice. You will see the `Failed to load message Msg#0 for account` error in the console and the chat will be empty. The bug was introduced in 7a968b6 (#4529). The added `messageId > C.DC_MSG_ID_LAST_SPECIAL` in the first `if` is purely for consistency and performance. It's the second one that matters.
432bff1
to
5b2901a
Compare
This is software development. It's impossible to foresee everything, even with colleagues reviewing the code etc. Things like this happens, even to the best developers :) |
This could cause the message list to not update on some events, but I have not seen a concrete example.The bug was introduced in 7a968b6.
(#4529).
The added
messageId > C.DC_MSG_ID_LAST_SPECIAL
in the firstif
is purely for consistency and performance.
It's the second one that matters.
Update: OK, this really does appear like this is causing the "empty chat issue". Reproduction steps are roughly as follows:
You will see the
Failed to load message Msg#0 for account
error in the console and the chat will be empty.I apologize for such carelessness.