Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML formatted links with numbers are wrongly formatted #2291

Closed
witchent opened this issue Jan 25, 2024 · 19 comments · Fixed by #2879
Closed

HTML formatted links with numbers are wrongly formatted #2291

witchent opened this issue Jan 25, 2024 · 19 comments · Fixed by #2879
Labels
A-Event Rendering How events are shown in the timeline O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect Something isn't working: bugs, crashes, hangs and other reported problems

Comments

@witchent
Copy link

Steps to reproduce

  1. Send an html formatted message including numbers in the displayed text like the one shown below (I used element web and /html for the test, but noticed the issue because I am using maubot rss reader)
  2. Look at the rendered message in element-x

Example:
/html <h1>Matrix: <a href=\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\">This Week in Matrix 2024-01-22</a></h1>
Source for maubot message:

{
  "type": "m.room.message",
  "sender": "@mxid",
  "content": {
    "msgtype": "m.notice",
    "body": "# Matrix: This Week in Matrix 2024-01-22 (https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/)",
    "format": "org.matrix.custom.html",
    "formatted_body": "<h1>Matrix: <a href=\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\">This Week in Matrix 2024-01-22</a></h1>\n"
  },
  "origin_server_ts": now,
  "unsigned": {
    "age": numbers
  },
  "event_id": "ev",
  "room_id": "room"
}

Source for element-web message sent via html

{
  "type": "m.room.message",
  "sender": "@mxid",
  "content": {
    "msgtype": "m.text",
    "format": "org.matrix.custom.html",
    "body": "<h1>Matrix: <a href=\\\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\\\">This Week in Matrix 2024-01-22</a></h1>",
    "formatted_body": "<h1>Matrix: <a href=\\\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\\\">This Week in Matrix 2024-01-22</a></h1>",
    "m.mentions": {}
  },
  "origin_server_ts": now,
  "unsigned": {
    "age": small number,
    "transaction_id": "more numbers"
  },
  "event_id": "$ev",
  "room_id": "roomid"
}

Outcome

What did you expect?

The url part of the message should be clickable and link to the correct webpage

What happened instead?

Most of the message is not clickable, only the text is clickable and links the numbers (so opens phone app on my phone) instead of the webpage.
See picture
Screenshot_Element X nightly

Your phone model

Oneplus 6

Operating system version

Android 14

Application version and app store

Element-x-0.4.2-nightly

Homeserver

Synapse 1.99

Will you send logs?

No

Are you willing to provide a PR?

No

@witchent witchent added the T-Defect Something isn't working: bugs, crashes, hangs and other reported problems label Jan 25, 2024
@jmartinesp
Copy link
Member

Looks like an issue with Linkify. The odd thing is we actually had some measures to prevent already linked text from being replaced, we must have missed something.

@jmartinesp jmartinesp added S-Minor Impairs non-critical functionality or suitable workarounds exist O-Occasional Affects or can be seen by some users regularly or most users rarely A-Event Rendering How events are shown in the timeline labels Jan 26, 2024
@jmartinesp
Copy link
Member

I had another chance to look at this, and it's caused by the href attr having the wrong format.

If you take a look, when the backslash is added here:

"formatted_body": "<h1>Matrix: <a href=\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\">This Week in Matrix 2024-01-22</a></h1>

It'll actually generate:

"formatted_body": "<h1>Matrix: <a href=\\\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\\\">This Week in Matrix 2024-01-22</a></h1>"

Which isn't recognised by the Rust SDK as valid HTML (I guess?) because what we receive in the app is a plain:

<h1>Matrix: This Week in Matrix 2024-01-22</h1>

Which is why the date is automatically linkified there. I tried sending the same HTML without the backslash and it was parsed just fine:

image

Top: with backslash.
Bottom: without it.

@witchent
Copy link
Author

I tried sending the following in element-web:
/html <h1>Matrix: <a href="https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/">This Week in Matrix 2024-01-22</a></h1>

It still shows the same in element-x (i.e. wrong linkified). Can you tell me what you did to get the correct link?

@jmartinesp
Copy link
Member

I tried sending the following in element-web: /html <h1>Matrix: <a href="https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/">This Week in Matrix 2024-01-22</a></h1>

It still shows the same in element-x (i.e. wrong linkified). Can you tell me what you did to get the correct link?

I didn't do anything special, I just sent that same command using Element Web (the nightly version, I don't know if that can make a difference). In fact, I just retried sending the command you attached in this message and it got parsed fine too.

I'm also testing this using a nightly version the EXA app, but as far as I know no changes were done to the HTML parsing, so it shouldn't be different than using the stable version.

@witchent
Copy link
Author

witchent commented Feb 13, 2024

Thats super weird. My version was compiled just yesterday from the source (using the fdroid-variant, but it did the same back when I was using the nightly apk from firebase).
Anything I can do to debug this further?
The source body (sending it like in the last message) is the same as the one maubot creates, i.e. there is one backslash before each ", but still it gets rendered without the link

Edit:// I just sent aforementioned command via develop.element.io, same thing happened

@jmartinesp
Copy link
Member

jmartinesp commented Feb 14, 2024

Anything I can do to debug this further?

Maybe you could enable 'view source' in Settings > Advanced settings, the long click on the messages and paster their JSON sources here.

FWIW, I tried manually sending a raw event with the same content as maubot according to your initial message from EW using the dev tools and it was rendered fine, both with the current code and a nightly from a couple of weeks ago:

// Content
"content": {
  "msgtype": "m.notice",
  "body": "# Matrix: This Week in Matrix 2024-01-22 (https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/)",
  "format": "org.matrix.custom.html",
  "formatted_body": "<h1>Matrix: <a href=\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\">This Week in Matrix 2024-01-22</a></h1>\n"
}

// JSON event source in the app:
{
    "content": {
        "body": "# Matrix: This Week in Matrix 2024-01-22 (https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/)",
        "format": "org.matrix.custom.html",
        "formatted_body": "<h1>Matrix: <a href=\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\">This Week in Matrix 2024-01-22</a></h1>\n",
        "msgtype": "m.notice"
    },
    "event_id": "$RBIBKiUnmsgs4mcysaj7RdI660PJ50TqfW-jJGsuqXY",
    "origin_server_ts": 1707897867882,
    "room_id": "roomid",
    "sender": "userid",
    "type": "m.room.message",
    "unsigned": {}
}

Result:

image

So I'm not sure what could be causing the strange behaviour on your side, to be honest.

@witchent
Copy link
Author

witchent commented Feb 14, 2024

Okay so I used develop.element.io right now, to send (from another account, in an encrypted room, with rich editor disabled) the message /html <h1>Matrix: <a href="https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/">This Week in Matrix 2024-01-22</a></h1>.

Element-X Android (fdroid version, compiled from develop tonight) renders it as in the screenshot before (so wrong), the resulting source reads (after passing it through jq):

{
  "content": {
    "body": "<h1>Matrix: <a href=\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\">This Week in Matrix 2024-01-22</a></h1>",
    "format": "org.matrix.custom.html",
    "formatted_body": "<h1>Matrix: <a href=\"https://matrix.org/blog/2024/01/22/this-week-in-matrix-2024-01-22/\">This Week in Matrix 2024-01-22</a></h1>",
    "m.mentions": {},
    "msgtype": "m.text"
  },
  "event_id": "$seroZ2DXR6mIbSv8Gh9lx-U6BLLfBUQ3HarNgGIEq98",
  "origin_server_ts": 1707899880584,
  "room_id": "roomid",
  "sender": "userid",
  "type": "m.room.message",
  "unsigned": {}
}

So funnily enough, here it seems the body does not get rendered correctly (not converted to markdown I guess?), but I don't think that that's the problem (and I don't know why this happens either). If you have no idea and no one else can reproduce this it's gonna be a hard find I guess..

@djmaze
Copy link

djmaze commented Feb 27, 2024

Same problem here with latest Element X Android nighly (via Firebase app distribution).

@vincejv
Copy link

vincejv commented May 17, 2024

Element X for iOS doesn't have this issue, despite "sharing the same codebase" details on dupe ticket: #2871

@vincejv
Copy link

vincejv commented May 17, 2024

Which is why the date is automatically linkified there. I tried sending the same HTML without the backslash and it was parsed just fine:

So I'm not sure what could be causing the strange behaviour on your side, to be honest.

@jmartinesp doesn't make sense when iOS isn't having the same issue? I've noticed it happens because the numbers are detected as tel: links. Maybe it's specific to a specific version of Android or Android Skin? I didn't experience the same on Element X iOS as stated on #2871

@jplatte
Copy link
Contributor

jplatte commented May 17, 2024

I can reliably reproduce this in EX 0.4.12, including on my own messages, and it's been this way for a long time.

I'm using LineageOS 20-20240508-NIGHTLY-FP3.

The data model from the SDK is clearly not the problem, the SDK doesn't do link detection in the first place.

Reproducer: write a markdown message like [test 123](https://example.com). See that it creates a tel link on the number, no link to example.com.

@jmartinesp
Copy link
Member

@jplatte I just tested sending a MD message from EW with your reproducer [test 123](https://matrix.org) and when I clicked on it on EXA (latest version) it opened the URL, it didn't autolinkify the phone number. Which app did you send the message from? Or better yet, can you share the actual event source JSON?

@jmartinesp
Copy link
Member

Also @vincejv I tried sending the same message that on your dupe issue, first just the HTML and then the whole event contents manually in case the MD <-> HTML mismatch could somehow cause this, but I always got the same result, which isn't 100% formatted right, but I can't reproduce the issue either:

image

I'd be glad to fix it, but I just can't reproduce it so I don't know what's causing it...

@Xiretza
Copy link

Xiretza commented May 18, 2024

The following event source, generated by Element Desktop 1.11.65, shows up as a plaintext test plus a phone number linkified 123 in EXA 0.4.12:

{
  "type": "m.room.message",
  "content": {
    "msgtype": "m.text",
    "body": "[test 123](https://matrix.org)",
    "format": "org.matrix.custom.html",
    "formatted_body": "<a href=\"https://matrix.org\">test 123</a>",
    "m.mentions": {}
  }
}

@vincejv
Copy link

vincejv commented May 18, 2024

in case the MD <-> HTML mismatch could somehow cause this

hello @jmartinesp thanks for your prompt reply, I'm not working with any markdown payload nor converting MD->HTML, i'm all sending them as raw html. I'm using the plain old matrix client api to send these messages, please use the curl reproducer below to reproduce these 2 issues in a single message

cUrl reproducer:

curl -X POST "https://{server_url}/_matrix/client/r0/rooms/{roomId}/send/m.room.message?access_token={accessToken}" \
     -H "Content-Type: application/json" \
     -d '{
           "msgtype": "m.text",
           "body": "This is 123-204 the plain text message",
           "format": "org.matrix.custom.html",
           "formatted_body": "<h3><a href='http://example.com'>This is 123-204</a></h3>the HTML message"
         }'

Element X for Android:

image

Element X for iOS:

image

*Edit: Fixed formatted_body, dangling tag

@jmartinesp
Copy link
Member

@vincejv I just tried with your cURL reproducer and it still worked fine for me, while testing on both an Android 13 emulator and an Android 14 device in v0.4.12:

right-link.mp4

I've tested it in debug, nightly and release variants of the app, enabling and disabling the RTE from advanced settings as well as other developer options, so I'm really confused about why I can't reproduce it at all, although I was already pretty sure none of these options should make any difference.

In any case I took a look at the code again and I think I might have found a possible cause for the issue you see, although it still doesn't explain why it's working fine in my case. Next week I'll modify the code a bit and create a debug version you can test and check if the problem is gone.

@jmartinesp
Copy link
Member

Could any of the people who had issues confirm if #2879 (comment) fixes it?

@witchent
Copy link
Author

Could any of the people who had issues confirm if #2879 (comment) fixes it?

Yes it does for me, thanks a lot for tracking this down, it was driving me crazy :D

@jplatte
Copy link
Contributor

jplatte commented May 24, 2024

0.4.13 is out with the fix! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Event Rendering How events are shown in the timeline O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect Something isn't working: bugs, crashes, hangs and other reported problems
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants