element-hq / element-android

A Matrix collaboration client for Android.
https://element.io/
Apache License 2.0
3.31k stars 702 forks source link

Forceful conversion to numbered lists in messages causes messages to change in meaning #4777

Closed Avamander closed 2 years ago

Avamander commented 2 years ago

Steps to reproduce

  1. Someone asks me a question:
    
    We have three choices here:
  2. Pizza with pineapple
  3. Sushi
  4. Ramen

What do you want to eat?


2. I answer: `2. sounds good`
3. I'm brought pizza with pineapple because Element Android changed my `2.` to `1.` due to its internal conversion to numbered list (with a single item)

This is **incredibly** awful.

### Outcome

#### What did you expect?
That my message is displayed how it was sent.

#### What happened instead?
The meaning of my message was changed.

### Operating system version

Android 10

### Application version and app store

1.3.9

### Homeserver

matrix.org
ouchadam commented 2 years ago

this looks like a missing feature with the markdown/html library we use, have raised an issue https://github.com/noties/Markwon/issues/381

notramo commented 2 years ago

This is standard Markdown feature (automatically numbering ordered list). Maybe disable ordered lists in Element?

basxto commented 2 years ago

Biggest issue is that Element Desktop and Element Android handle this differently, so the answer will look different with different clients. Element Desktop also makes it into a list, but it does not do automatical numbering.

List decorations are also dropped in the Direct Messages overview, there the answer would be just rendered as sounds good

HarHarLinks commented 2 years ago

it is really horrible when you try to make an appointment and e.g. suggest "4. April" and depending whether the other person(s) use(s) element web, android, or iOS, they see "1. April". Then suddenly on 1. April you get a call why you're late.

daniellekirkwood commented 2 years ago

Have triaged on the WTFs board, will remove myself now đź‘Ť

Seil0 commented 2 years ago

How is this only a minor issue? This makes element unusable on Android.

notramo commented 2 years ago

Because if sponsor reports issue, then it's a serious issue despite if it doesn't even affects the UX (or very minimally - see confetti animation or typing indicator redesign). If somebody other reports it, who doesn't pay, then it's a minor issue, despite it can be so serious that users migrate back to proprietary chat platforms. See the #2504 call glare bug, which repeatedly makes users migrate back to Facebook™ Messenger™, but the Element Android don't give a s*** for since 2 years, despite it's probably 30 lines of code and probably can be fixed in 20 minute by somebody who is familiar with the codebase (I unfortunately don't know Kotlin, and I'm not familiar with Android development). So don't expect a fix for this bug either. Use a client that's developed by volunteers, not a company (Syphon on mobile, and Cinny on desktop). They probably consider these kind of bugs more important.

ouchadam commented 2 years ago

to help clear up the labelling, this issue was originally marked as minor because "a suitable workaround exists", which in our case is disabling the markdown preference and the markdown setting is not enabled by default.

however... it seems this is not actually the case and even without the markdown toggle disabled the list syntax is being unexpectedly processed, will upgrade the severity

ouchadam commented 2 years ago

Fixed in v1.4.28

Avamander commented 2 years ago

@ouchadam

I think that fix would still change the meaning of messages in some scenarios, especially if the behaviour is not unified between platforms.

For example:

What were the deadlines for submission?
Let me check my calendar real quick
4. August
17. August

Would turn the last message into:

4. August
5. August
ouchadam commented 2 years ago

unfortunately there's no avoiding the number change when markdown is enabled and the list syntax is used (github behaves the same), I can confirm web and android are in sync at least!

Sending the following messages...

5. five
10. ten
/plain
5. five
10. ten
WEB ANDROID
2022-07-08T10:09:34,188827306+01:00 2022-07-08T10:10:51,736325070+01:00
Seil0 commented 2 years ago

The conversion fix is still not ideal for a normal user who just wants to send plain text messages. But at least it consistent between the platforms now, so errors should be easier to spot.