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

MUC read receipts causing empty lines #1442

Closed
laszlovl opened this issue Feb 11, 2019 · 7 comments

Comments

@laszlovl
Copy link
Contributor

commented Feb 11, 2019

The client is Chatsecure, so it could very well be that the format receive is invalid. Still, there shouldn't be any empty lines because of it and I'm sure this is a regression compared to an older Converse version.

bug

The XML we receive:

<message xml:lang="en" to="user@domain.com/converse.js-57308413"
         from="room@conference.domain.com/user" type="groupchat" xmlns="jabber:client">
    <received xmlns="urn:xmpp:receipts" id="DC545AFB-FFC3-434C-BB6A-22F9F5069518"/>
    <origin-id xmlns="urn:xmpp:sid:0" id="CE08D448-5ED8-4B6A-BB5B-07ED9DFE4FF0"/>
</message>
@laszlovl

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2019

I suspect e1f8d53

@jcbrand

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

Hmm, I thought I fixed this recently. Are you on the latest commit on the master branch?

@jcbrand

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

Maybe I only fixed it for private chats...

@laszlovl

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2019

Yeah, this is in the latest master. So far what I've got is that handleReceipt() isn't even called for these messages. Yet it generates a normal looking HTML:

<div class="message chat-msg groupchat chat-msg--followup" data-isodate="2019-02-11T18:34:30+01:00" data-msgid="" data-from="room@conference.domain.com/user" data-encrypted="">
@jcbrand

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

So far what I've got is that handleReceipt() isn't even called for these messages. Yet it generates a normal looking HTML:

Yes, IIRC I only added this for one-one-one chats, probably with the intention of still adding it to MUCs. I've had a crazy-busy last two weeks.

@laszlovl

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2019

Right, I mostly figured it out. The message isn't handled by converse-chatboxes onMessage, but by converse-muc onMessage. The latter currently doesn't check for receipts at all, if I copy & paste some code into it, like

const receipt = sizzle(`received[xmlns="${Strophe.NS.RECEIPTS}"]`, stanza).pop();
                    if (receipt) { return }

it does detect that the message is a receipt.

Any thoughts on how to fix this nicely without duplicating code (and why the problem didn't appear to exist in previous versions? The commit I linked doesn't really explain that since mucs were left untouched)

@jcbrand

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

Any thoughts on how to fix this nicely without duplicating code

handleReceipt needs to be called for MUC messages as well. ChatRoom inherits handleReceipt from ChatBox, so it can be called on the chatroom object.

laszlovl added a commit to laszlovl/converse.js that referenced this issue Feb 12, 2019

@jcbrand jcbrand closed this in dbcf600 Feb 12, 2019

@jcbrand jcbrand added this to the 4.1.1 milestone Feb 12, 2019

jcbrand added a commit that referenced this issue Feb 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.