element-hq / riot-android

A glossy Matrix collaboration client for Android
Apache License 2.0
1.4k stars 394 forks source link

Messages containing only digits followed by a period are invisible #2285

Open dkasak opened 6 years ago

dkasak commented 6 years ago

This only happens when the message is sent from Riot Web with Markdown input turned on, but Riot Web itself displays the message properly. The problem seems to be that it also produces a formatted_body containing an ordered list with a single empty item, which Riot Android then renders as an empty message.

This was tested on Riot Android 0.8.8.

bmarty commented 6 years ago

Bug confirmed

bmarty commented 6 years ago

(thanks for reporting)

bmarty commented 6 years ago

The pb is also present on iOS (https://github.com/vector-im/riot-ios/issues/1897)

zeratax commented 5 years ago

This seems related, but writing a message like:

`5. October 😊`

results in:

while desktop isn't technically a bug it's still probably not what users would expect to happen or necessarily understand why a date is being replaced by a different date. I can create a separate issue for this for mobile and web if that's wanted.

screenshot_riot im_20181015-233520

bmarty commented 5 years ago

It's related to Markdown formatting, so, despite the huge inconvenient, we have to say it's a feature, not a bug... Also note that 123456. will not be displayed correctly as well on Riot-Web

dkasak commented 5 years ago

I disagree, this is definitely a bug, but it's also not happening anymore on Riot Android 0.8.17 so it seems like it has been (inadvertently?) fixed! It now renders as 1., just like it should. I would argue it is still semantically incorrect, though, and that numbered lists with a single, empty item should be special-cased and not recognized as Markdown at all, but that is a Riot Web bug.

The wrong rendering of 123456. in Riot Web is a totally separate bug, judging by the result:

bug

zeratax commented 5 years ago

Alright even if we don't care about markdown not being consistent and not doing what users probably would expect, I'm still very confused about the emoji getting mangled.

bmarty commented 5 years ago

I still have the issue with 1.. Maybe you have inadvertently disabled the markdown?

dkasak commented 5 years ago

I don't think so.

Riot Web:

Riot Web

Message source (as seen in Riot Web):

{
  "origin_server_ts": 1539892403760,
  "sender": "@dkasak:termina.org.uk",
  "event_id": "$15398924031747tJKKp:termina.org.uk",
  "unsigned": {
    "age": 71,
    "transaction_id": "m1539892405307.30"
  },
  "content": {
    "body": "1.",
    "msgtype": "m.text",
    "formatted_body": "<ol>\n<li></li>\n</ol>\n",
    "format": "org.matrix.custom.html"
  },
  "type": "m.room.message",
  "room_id": "!DCRUrRQxGhHTRwlYla:termina.org.uk"
}

Riot Android: Riot Android

Message source (as seen in Riot Android): Riot Android source

Riot Web version: 0.16.5 Riot Android version: 0.8.17 (G-b155)

dkasak commented 5 years ago

So could this be reopened? It works in one client and it doesn't in another. It was also working in both clients for a bit as my screenshots show.

I also see a sensible workaround: don't consider empty lists as something the user intended and fall back on body in those cases. However, it might be more appropriate to fix it in Riot Web, such that it doesn't actually emit HTML at all when you type in ^[0-9]+\.\s*$ (or something like that) even in Markdown mode. In fact, this is what Riot Android does.

Should I open this issue on Riot Web?

bmarty commented 5 years ago

I reopen the issue to not forget it, but I'm not sure what I can do for it. Feel free to open an issue on Riot Web if you think it's appropriate.