Skip to content
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

Bug Fix #1938 #2019

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

OdenDillenkoffer
Copy link

For the bug reported in #1938 there seems to be an issue of not grabbing the right time stamp. This is similar to the bug in #1190 of not grabbing the proper delay for MAM messages. To fix this, instead of discarding the original timestamp and trying to find the oldest delay, which may have been lost to a server quirk, we preserve the timestamp until an older delay is found. Then, and only then, will the timestamp update. Of course if it still can't find a delay and there is no timestamp, the fall back of grabbing the current time is still available. This should fix the bug experienced in #1938 and #1190 , given they look like they stem from a similar issue.

  • I ran valgrind when using my new feature

How to test the functionality

  • Step 1 - Profanity is not connected.
  • Step 2 - Receive messages in a MUC
    *Step 3 - Connect Profanity
    Now the timestamps should be correct for the MUC.

Previous to this there were issues of inaccurate time stamps, but now that should be mitigated.

Copy link
Member

@jubalh jubalh left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.
Before I look at the code itself. Please make sure you follow.
So first thing you have to do is find the contributing.md file. Read it.
Then you can check other PRs and see what they look like.

You describe the PR as comments in the code. You will find out that nobody else is doing this :) Use the body of the git commit message.

Once you are done you can click on request review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants