ubports / messaging-app

Messaging App for Ubuntu Touch devices. Moved to GitLab.
https://gitlab.com/ubports/development/core/messaging-app
Other
19 stars 20 forks source link

Not showing full text of message #339

Closed jezek closed 2 years ago

jezek commented 2 years ago

I got an text message. In notifications I saw that the message contains some words, but when clicked and opened in messaging-app the message is not shown whole.

Here is the (redacted) message as saved in the ~/.local/share/history-service/history.sqlite:

sqlite> select messageStatus, * from text_events order by eventId desc limit 1;
0|ofono/ofono/account0|+421910123456|2022-05-14T22:16:41+0200-1|+421910123456|2022-05-14T20:16:43.000Z|0|)@<¥}3m Text! this should be visible but is not ;)|0|0|2022-05-14T20:16:44.646Z||0|2022-05-14T20:16:41.000Z

The text message should be: )@<¥}3m Text! this should be visible but is not ;) but instead this is shown (all text in picture is redacted/censored except the last message bubble): screenshot20220514_223236579

lduboeuf commented 2 years ago

humm, can't repeoduce on RC

lduboeuf commented 2 years ago

if any issue regarding text, it is probably here: https://github.com/ubports/messaging-app/blob/xenial/src/qml/MessageBubble.qml#L61

jezek commented 2 years ago

I looked at the code of the parseText function (just looked, not tested) and it seems that this line, which removes html tags is our culprit. At least the regex tester site says so.

This brings me to the question why is the message text stripped from html tags? What is someone wants to send you a text message which explains some tags or to use the < char?

I think the proper way is not to remove the tags, but to escape all the html characters.

Note: There is another parseText function in the messaging-app, which does escaping instead of string but is missing escaping the &, " and ' characters. And there is also a fishy if statement which seems to do nothing. Otherwise these 2 functions are in principle the same and maybe should be merged into one.

Note2: Why if some linkifyable text is found, the phone numbers resp. account names are not linkifyed resp. replaced?

lduboeuf commented 2 years ago

I looked at the code of the parseText function (just looked, not tested) and it seems that this line, which removes html tags is our culprit. At least the regex tester site says so.

This brings me to the question why is the message text stripped from html tags? What is someone wants to send you a text message which explains some tags or to use the < char?

idk the initial reason ;-), we should try as you said to escape them.

Note2: Why if some linkifyable text is found, the phone numbers resp. account names are not linkifyed resp. replaced?

Sorry, don't understand

jezek commented 2 years ago

Note2: Why if some linkifyable text is found, the phone numbers resp. account names are not linkifyed resp. replaced?

Sorry, don't understand

For the sake of explanation, let's divide the parseText function into some parts.

  1. Text sanitation: https://github.com/ubports/messaging-app/blob/2c273ab5f1eae1e305da14d3516f70b53b6deda5/src/qml/MessageBubble.qml#L66-L71
  2. Web links detection & turning them into html links: https://github.com/ubports/messaging-app/blob/2c273ab5f1eae1e305da14d3516f70b53b6deda5/src/qml/MessageBubble.qml#L72-L77
  3. The phone number detection & turning them into html links: https://github.com/ubports/messaging-app/blob/2c273ab5f1eae1e305da14d3516f70b53b6deda5/src/qml/MessageBubble.qml#L78-L84
  4. Accounts detection & highlighting them: https://github.com/ubports/messaging-app/blob/2c273ab5f1eae1e305da14d3516f70b53b6deda5/src/qml/MessageBubble.qml#L85-L88
  5. Returning resulting sanitized linkified text: https://github.com/ubports/messaging-app/blob/2c273ab5f1eae1e305da14d3516f70b53b6deda5/src/qml/MessageBubble.qml#L89

And you can see in the part 2, there is a early return if the resulted linkified text is different as input text. Which means, if the text contains some string which can be turned into web link, than the function will not continue and not make phone numbers clickable (part 3), nor accounts highlighted (part 4). And that is a (small) problem. If the text contains only a web link or only a phone number, they will be made clickable. But if the text contains both of them, only the links will be clickable. I think we would want to have both clickable. Or not?

lduboeuf commented 2 years ago

I see thx, well idk if this is intentional. It reminds me Teleport where if you have a Video and a Link, you cannot use both. Some tests need to be done, maybe if you have some time, you can play with it ?

jezek commented 2 years ago

I see thx, well idk if this is intentional. It reminds me Teleport where if you have a Video and a Link, you cannot use both.

The first issue that comes to my mind is when there is a number inside linkifyable text, which could be identified as phone number. Eg. Some text and linkifyable.org/number/00420901123456/etc and more text, then the linkify part (part 2) would duplicate the number (Some...<a href="linkifyable.org/...004209...">linkifyable.org/...004209...</a> and more) and then the number linkify part (part 3) would wreck havoc inside the tags.

Some tests need to be done,

That's what I thought. First some test code should be done, with some text snippets and the resulting counterparts. And then the parseText function should be tweaked to pass the tests.

maybe if you have some time, you can play with it ?

Yes, I can. But first I have/want to bring my other work-in-progress project to end (or at least to a checkpoint), which can last a few weeks.

jezek commented 2 years ago

Fixed by #341

Flohack74 commented 2 years ago

@jezek can you re-test this with latest RC? I ran another RC update now, to bring in updated messaging-app and I want to make sure we have it for next Tue release of OTA-24

jezek commented 2 years ago

@Flohack74 I just updated to latest RC (v143, 2022-W25) and tested (looked at) the incriminating message. The message is displayed whole, nothing is missing. This issue is still fixed.

I've also tried to send myself a SMS with text like this:

test & <div> @#*'  </div> /\~"«`»“§ „€£$¥₹”
戻す
<img href="aaa" />

Everything looks correctly. No issue found.

PS: Please, ignore my mistake in the test message, the img tag has src attribute instead of href. ;)

Flohack74 commented 2 years ago

thx! all good !