fix to enable links in markdown
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):
🖼️ Screenshots
| 🏚️ Before | 🏡 After |
|---|---|
🏁 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?)
/backport to stable-18.0
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..
when work on this PR is continued, also have a look at https://github.com/nextcloud/talk-android/issues/3634
Codacy
Lint
| Type | master | PR |
| Warnings | 81 | 81 |
| Errors | 126 | 126 |
SpotBugs
| Category | Base | New |
|---|---|---|
| Bad practice | 6 | 6 |
| Correctness | 11 | 11 |
| Dodgy code | 84 | 84 |
| Internationalization | 3 | 3 |
| Malicious code vulnerability | 3 | 3 |
| Performance | 6 | 6 |
| Security | 1 | 1 |
| Total | 114 | 114 |
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/3481-talk.apk
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.
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) |
I personally vote to go with option2, so markdown links are working and all other content can be copied manually if needed.