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

Add url in body of oob messages #69

Merged
merged 1 commit into from
Jan 20, 2023
Merged

Add url in body of oob messages #69

merged 1 commit into from
Jan 20, 2023

Conversation

Ppjet6
Copy link
Contributor

@Ppjet6 Ppjet6 commented Jan 19, 2023

This is an attempt at fixing http uploading images sent as OOB messages. I'm submitting this because I noticed multiple clients aren't displaying images when sent from xmpp-web.

https://docs.modernxmpp.org/client/protocol/#communicating-the-url

I have to admit I haven't tested this change but it seems rather straightfoward. This does means that a body can't be provided alongside a URL.

Signed-off-by: Maxime “pep” Buquet pep@bouah.net

Signed-off-by: Maxime “pep” Buquet <pep@bouah.net>
@nioc
Copy link
Owner

nioc commented Jan 19, 2023

Hello @Ppjet6, thanks for this proposal. I'll look at it this week-end.
I did not have display issue with Gajim, Dino (Linux) or Conversations (Android) but if my implementation does not fit XMPP standards and causes user issue, we have to fix it.

Your change seems straightfoward, but I prefer to check it before merging 😉

@nioc nioc added the bug Something isn't working label Jan 19, 2023
@nioc nioc merged commit 58cd025 into nioc:master Jan 20, 2023
@nioc
Copy link
Owner

nioc commented Jan 20, 2023

As you made me opening code for others issues, I check this one too and you were right (may be a regression since the move from Stanza to @xmpp/client in 0458da7)
Thanks for the report and more importantly for the PR, you're the first 💓 👏🏻

@nioc nioc self-requested a review January 20, 2023 01:10
@nioc
Copy link
Owner

nioc commented Jan 20, 2023

0.9.4 released and pushed on dockerhub 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants