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

Linkified text is too greedy and gobbles punctuation #4208

Open
edent opened this issue Jan 28, 2025 · 9 comments · May be fixed by #4214
Open

Linkified text is too greedy and gobbles punctuation #4208

edent opened this issue Jan 28, 2025 · 9 comments · May be fixed by #4214
Labels
A-Event Rendering How events are shown in the timeline O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect Something isn't working: bugs, crashes, hangs and other reported problems

Comments

@edent
Copy link

edent commented Jan 28, 2025

Steps to reproduce

Image

Outcome

What did you expect?

Punctuation at the end of a URl shouldn't be included in that URl.

What happened instead?

The link goes to a 404.

I suspect this is related to this Android bug - https://issuetracker.google.com/issues/161727315

Your phone model

Google Pixel 8Pro

Operating system version

Android 15

Application version and app store

Element X 0.7.6 Play Store

Homeserver

matrix.org

Will you send logs?

No

Are you willing to provide a PR?

No

@edent edent added the T-Defect Something isn't working: bugs, crashes, hangs and other reported problems label Jan 28, 2025
@codesalatdev
Copy link

codesalatdev commented Jan 28, 2025

Not to be that guy, but parenthesis and punctuation are perfectly valid parts of a well-formed URI as described by RFC3986

While I personally agree with the fact that it's really annoying to type, the RFC also states that:

In practice, URIs are delimited in a variety of ways, but usually
within double-quotes "http://example.com/", angle brackets
<http://example.com/>, or just by using whitespace

Take the search query "symbol )" on duckduckgo as an example: https://duckduckgo.com/?t=h_&q=symbol+). Not including the parenthesis in the URI would alter the search results significantly.

@edent
Copy link
Author

edent commented Jan 28, 2025

I don't experience this behaviour in Slack or Teams for Android.

You can user-blame all you want, but I don't think that will attract more participants to your platform. Your response makes me a lot less likely to use or recommend Element.

@JadedBlueEyes
Copy link

Not to be that guy, but parenthesis and punctuation are perfectly valid parts of a well-formed URI as described by RFC3986

While I personally agree with the fact that it's really annoying to type, the RFC also states that:

In practice, URIs are delimited in a variety of ways, but usually
within double-quotes "http://example.com/", angle brackets
<http://example.com/>, or just by using whitespace

Take the search query "symbol )" on duckduckgo as an example: https://duckduckgo.com/?t=h_&q=symbol+). Not including the parenthesis in the URI would alter the search results significantly.

Auto-linkification is a convenience feature. It doesn't need to follow a spec, it should correctly highlight as many links as possible.

From my experience, people commonly place punctuation immediately after a link, and rarely post a link ending in such symbols.
In the more unusual case where they do need to share a link, they can make an explicit link with markdown syntax, etc.

@spaetz
Copy link

spaetz commented Jan 28, 2025

To be fair, specwise this is a valid URL: https://en.m.wikipedia.org/wiki/Wikipedia_(disambiguation) (and this markdown parser displays it as such).

Perhaps, this can be made smart enough so that it checks for open and unclosed parenthesis before the URl, to be safe.

This is what WhatsApp does.

Image

Image

@JadedBlueEyes
Copy link

Screenshot_20250128-190156.png

WhatsApp android seems to exclude the closing bracket only when the URL is immediately preceded by an opening bracket.

The Wikipedia app, however, URL encodes the brackets when you share a page - eg https://en.wikipedia.org/wiki/Wikipedia_%28disambiguation%29?wprov=sfla1

@spaetz
Copy link

spaetz commented Jan 28, 2025

Interesting, MY WhatsApp Android did manage this correctly (see last screenshot above your post). However, I did manage to confuse it in the end

Image

In general, I was quite impressed though, how smart it handled these cases.
Wiki might URL encode parentheses when the hitting share button, but when directly copying links within wiki, they include parens verbatim eg https://en.m.wikipedia.org/wiki/Wikipedia_(disambiguation) contains the verbatim link https://en.m.wikipedia.org/wiki/Wiki_(disambiguation)

Anyway, up to someone else on how to deal with this mess 😜

@codesalatdev
Copy link

I don't experience this behaviour in Slack or Teams for Android.

You can user-blame all you want, but I don't think that will attract more participants to your platform. Your response makes me a lot less likely to use or recommend Element.

My comment seems to have caused emotional distress, which is not what I was trying to achieve. Let me re-phrase what I said earlier with less words:

  1. It's really annoying to fix up links garbled by punctuation
  2. Punctuation at the end of an URL is valid, so we cannot just ignore it like you suggested

Hope that makes sense. I am not trying to blame "the user", nor do I care whether you use Element or not.

WhatsApp android seems to exclude the closing bracket only when the URL is immediately preceded by an opening bracket.

I really like this behavior, although again the URL could be preceded by other text which is also inside the brackets: blablabla (see https://wikipedia....).

How about detecting links that have been pasted (if possible) and not adding symbols typed by the user to the link?

@jmartinesp
Copy link
Member

FWIW we're using Google's official linkification solution for Android, so I expected it to handle these cases a bit better, to be honest.

We can try looking for alternative solutions, since there may be some as performant and more correct.

@jmartinesp jmartinesp added S-Minor Impairs non-critical functionality or suitable workarounds exist O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience A-Event Rendering How events are shown in the timeline labels Jan 29, 2025
@jmartinesp
Copy link
Member

Huh, fun stuff: if you add an URL without / characters the parenthesis are correctly excluded, but once it contains a / it's broken.

Image Image

@jmartinesp jmartinesp linked a pull request Jan 29, 2025 that will close this issue
9 tasks
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-Frequent Affects or can be seen by most users regularly or impacts most users' first experience 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.

5 participants