talk-android icon indicating copy to clipboard operation
talk-android copied to clipboard

fix to enable links in markdown

Open mahibi opened this issue 2 years ago • 9 comments

fix https://github.com/nextcloud/talk-android/issues/3633

fix to enable links in markdown when no top level domain was included in the link description.

Examples (without TLD):

Hacker News AlternativeTo Phoronix BNN-Breaking OSBA – Open Source Business Alliance

Examples (with TLD):

LinuxNews.de

🖼️ Screenshots

🏚️ Before 🏡 After
Bildschirmfoto vom 2023-11-30 11-02-24 geaendert Bildschirmfoto vom 2023-11-30 11-02-33 geaendert

🏁 Checklist

  • [x] ⛑️ Tests (unit and/or integration) are included or not needed
  • [x] 🔖 Capability is checked or not needed
  • [x] 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • [x] 📅 Milestone is set
  • [x] 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

mahibi avatar Nov 30 '23 10:11 mahibi

/backport to stable-18.0

mahibi avatar Nov 30 '23 10:11 mahibi

ah this destroys other links, e.g. "mailto:" and phone numbers.

As soon as app:textAutoLink is set to some value, markdown links with no top level domain in the link description fail to be shown as links.

Also app:textAutoLink="email|phone|map|web" or app:textAutoLink="email|phone|map" won't help.

Will have a closer look later..

mahibi avatar Nov 30 '23 10:11 mahibi

when work on this PR is continued, also have a look at https://github.com/nextcloud/talk-android/issues/3634

mahibi avatar Feb 12 '24 09:02 mahibi

Codacy

Lint

TypemasterPR
Warnings8181
Errors126126

SpotBugs

CategoryBaseNew
Bad practice66
Correctness1111
Dodgy code8484
Internationalization33
Malicious code vulnerability33
Performance66
Security11
Total114114

github-actions[bot] avatar May 07 '24 18:05 github-actions[bot]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/3481-talk.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud Talk app.

github-actions[bot] avatar May 07 '24 18:05 github-actions[bot]

So on one hand there is the need to support all markdown links, on the other hand there is automatic parsing.

It seems for now there is only the decision to enable or disable app:textAutoLink="all". If there are other solutions they might require quite some work. Also other messengers handle this quite differently and none of them supports all options. Especially with markdown (which most other messengers don't support) things seem to get more complicated.

Option1: with app:textAutoLink="all" Option2: without app:textAutoLink="all"
Markdown links are broken (Only when TLD is in the description, it's recognized as hyperlink. But the actual link from markdown might be different and is not taken into account). All other content is clickable if patterns match, but could also be senseless (esp. for phonenumbers) All markdown links are working as expected. Nothing else is recognized as clickable content (but link previews still work if a link was found)
grafik grafik

I personally vote to go with option2, so markdown links are working and all other content can be copied manually if needed.

mahibi avatar May 07 '24 18:05 mahibi