tinode / tinode-js

Tinode javascript bindings
Apache License 2.0
52 stars 42 forks source link

Sending emoji and link in the message breaks push notification #74

Closed dilshansdoq closed 4 months ago

dilshansdoq commented 6 months ago

Subject of the issue

Sending a message from user A to user B with the following text, such as Hello 😀, https://google.com/, doesn't send push notification to user B. Unit test of drafty also breaks if I add the above text here.

Your environment

Server-side

Client-side

Steps to reproduce

Send a text which has both an emoji and link in it.

Expected behaviour

Notification must be visible to other user.

Actual behaviour

drafty.go/Preview() errors out with error: fcm push: could not parse payload; invalid format and Hence notification is not sent.

Server-side log

I2024/03/13 12:05:24 in: '{"pub":{"id":"73980","topic":"usrCKgqP1WixEE","noecho":true,"head":{"mime":"text/x-drafty"},"content":{"txt":"Hello 😀, https://google.com","fmt":[{"at":10,"len":18}],"ent":[{"tp":"LN","data":{"url":"https://google.com"}}]}}}' sid='jw8Wfn_upvA' uid='m6fEsJf6KXk'

I2024/03/13 12:05:24 out: '{"ctrl":{"id":"73980","topic":"usrCKgqP1WixEE","params":{"seq":1596},"code":202,"text":"accepted","ts":"2024-03-13T06:35:24.186Z"}}' sid='jw8Wfn_upvA' uid='m6fEsJf6KXk'

W2024/03/13 12:05:26 fcm push: could not parse payload; invalid format

Client-side log

{"pub":{"id":"73980","topic":"usrCKgqP1WixEE","noecho":true,"head":{"mime":"text/x-drafty"},"content":{"txt":"Hello 😀, https://google.com","fmt":[{"at":10,"len":18}],"ent":[{"tp":"LN","data":{"url":"https://google.com"}}]}}} 
{"ctrl":{"id":"73980","topic":"usrCKgqP1WixEE","params":{"seq":1596},"code":202,"text":"accepted","ts":"2024-03-13T06:35:24.186Z"}}
or-else commented 6 months ago

The issue is on the client side in the javascript SDK, drafty should be "fmt":[{"at":~10~ 9,"len":18}]

This is the same issue as https://github.com/tinode/tindroid/issues/156

dilshansdoq commented 6 months ago

Can we use Intl.Segmenter to get the graphemes, which will count emoji as 1 character only. https://dev.to/ayc0/intlsegmenter-dont-use-stringsplit-nor-stringlength-dh6

Javascript: https://rosettacode.org/wiki/String_length#Unicode_grapheme_length Java: https://rosettacode.org/wiki/String_length#Grapheme_Length_7 (requires JDK20) Swift:https://rosettacode.org/wiki/String_length#Grapheme_Length_15

or-else commented 6 months ago
dilshansdoq commented 6 months ago
  • JS: yes, Intl.Segmenter is the way to go. It's not a small project though.

Yes you are right. So have you started working on it or should I give it a try.

I am not at all versed with java, maybe someone in the community can help with it.

  • Swift: it already works correctly.

Yes, it works in iOS.

or-else commented 6 months ago

So have you started working on it or should I give it a try.

I'm not working on it. You are welcome to do it.

This method https://github.com/tinode/tinode-js/blob/c6bbf30798991c5ad38acfabc4d157d6e9f21e61/src/drafty.js#L510 and the methods it calls should be updated.

I would split line here into graphemes and then pass into subsequent calls the array of graphemes instead of a string.

dilshansdoq commented 6 months ago

I have completed the implementation using Intl.Segmenter. I will be making a pull request soon.

I have one question tho, Intl.Segmenter is not yet supported on Firefox. How to deal with it. There is a npm library graphemer, It supports unicode 15 as well. It might be better to use this only. The code implementation at our side is not very different among these two libraries. We can switch to Intl.Segmenter once Firefox is supported as well. Let me know your thoughts on this.

or-else commented 6 months ago

If I understand correctly, Graphemer is not a polyfill for Intl.Segmenter, i.e. it has a different API.

We use format.js in TinodeWeb. It has a polyfill for Intl.Segmenter: https://formatjs.io/docs/polyfills/intl-segmenter/ https://www.npmjs.com/package/@formatjs/intl-segmenter Its API is identical to the official API and it's just slightly bigger than Graphemer. It's also well-designed (Facebook) and can be loaded only when needed.

or-else commented 6 months ago

Forgot to mention. Please fill out this form, if you have not done so already: https://github.com/tinode/chat/blob/master/docs/CLA.md

Thanks!