element-hq / element-x-android

Android Matrix messenger application using the Matrix Rust Sdk and Jetpack Compose
GNU Affero General Public License v3.0
1.07k stars 152 forks source link

Renders MSC mentions as call links #2914

Open Johennes opened 5 months ago

Johennes commented 5 months ago

Steps to reproduce

  1. Open a room where MSCs are mentioned
  2. Notice how the number after "MSC" is unhelpfully renderer as a call link

Screenshot_20240524-193158

Outcome

What did you expect?

No link

What happened instead?

A call link

Your phone model

Pixel 6

Operating system version

14

Application version and app store

0.4.12

Homeserver

matrix.org

Will you send logs?

No

Are you willing to provide a PR?

Yes

HarHarLinks commented 5 months ago

This is fixed for me as of 0.4.13

Johennes commented 5 months ago

Oh, brilliant! Closing this out then.

Johennes commented 2 months ago

I'm still seeing this on 0.5.0 actually.

share_5785505383077592500

jmartinesp commented 2 months ago

@Johennes could you confirm which locale (country + language) are you using in your device/for the app, if they're different? The 'linkification' of text messages uses an Android library that relies on locale, and I can't reproduce the issue with en_US.

HarHarLinks commented 2 months ago

Screenshot_20240821-093735_SchildiChat Next (Beta)_1

Using schildinext based on 0.4.16. android "languages" are 1) en_US and 2) de_DE.

jmartinesp commented 2 months ago

Using schildinext based on 0.4.16. android "languages" are 1) en_US and 2) de_DE.

Huh, that's weird. They're the same I have in my test device:

image

Although I'm testing it on v0.5.1. Also, I don't know if SchildiNext may have some different logic/post formatting for timeline messages.

I know it's kind of a pain but could you add links to some messages that have this issue, if they're public? Maybe they need to have some specific format.

HarHarLinks commented 2 months ago

https://matrix.to/#/!dSMpkVKGgQHlgBDSpo:matrix.org/$DkOnuCHUdwXR9ruNaNl_0C8xlDgGqlAjAyjdJUZYy5w?via=matrix.org&via=envs.net&via=element.io

jmartinesp commented 2 months ago

I can confirm I can't reproduce it on 0.5.1 (develop) with en_US and de_DE locales:

image

This is the linked message, but I don't see which content could be linkified (maybe the user123?):

image
jmartinesp commented 2 months ago

Could any of the affected users download a nightly EX version and test if it's working for them so we can confirm if the issue can be closed?

Johennes commented 2 months ago

I tried 0.5.1-nightly (40005010) and am seeing the same issue:

aaa

My languages are:

  1. English (US)
  2. English (UK)
jmartinesp commented 2 months ago

Ah, my bad, the behaviour changes based on the TelephonyManager outputs, that is: the phone number format expected for the country your SIM cards is registered in, if I'm not mistaken, so that's why I couldn't reproduce it even when changing the locale. From a related issue seen in the past:

Also, it was especially hard to debug because the issue was focused in phone number being linkified and the detection of those phone numbers depend on the region code returned by the TelephonyManager service, which will vary from one device to another. It's a lot easier to spot with emails, i.e.

I don't think there's an easy way to either test this on my end or modify the way the telephone links are detected... Maybe we could try iterating in the returned links and check whether they at least match a full number (so, non-letter character, a bunch of numbers, then another non-letter characters)... either that or we need to move to a new linkification library like this one.

Johennes commented 2 months ago

Maybe we could try iterating in the returned links and check whether they at least match a full number (so, non-letter character, a bunch of numbers, then another non-letter characters)

Something like that or maybe at least a minimum length? There are some public service phone numbers in Germany that are only 3 numbers. Any regular number I've seen so far was a lot longer though. With very short numbers, the advantage of tel-linking over manually punching in the number also seems rather small. So I think we wouldn't really lose much value if we'd only tel-link, say, >=7 digits.

Johennes commented 2 months ago

Just checked Signal and it does have the exact same problem for me. Ironically I've never noticed this, maybe because my conversations there are less technical and don't commonly include numbers or maybe because Signal's linking style is a lot more subtle than the one in Element X. 🤔

Screenshot_20240821-202826

It does really annoy me on Element X for some reason though. 🙈