Skip to content

Conversation

provokateurin
Copy link

The URL_PATTERN_AUTOLINK regex already allowed parenthesis from markdown, so the links were clickable, but there was no preview shown due to this regex not allowing them.

☑️ Resolves

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@provokateurin provokateurin added bug Something isn't working 3. to review Waiting for reviews labels Nov 16, 2024
@provokateurin
Copy link
Author

I'm not sure which milestones and branches are used in this repo, so feel free to backport however you see fit.

@provokateurin provokateurin force-pushed the fix/ncrichtext/url-pattern-markdown-parenthesis branch from b3858e2 to 013ae78 Compare November 16, 2024 10:30
@provokateurin provokateurin changed the title fix(NcRichTest): Allow markdown parenthesis in URL_PATTERN fix(NcRichText): Allow markdown parenthesis in URL_PATTERN Nov 16, 2024
@ShGKme ShGKme added this to the 8.20.1 milestone Nov 16, 2024
@ShGKme ShGKme added the feature: richtext Related to the richtext component label Nov 16, 2024
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, thanks

Copy link
Contributor

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs extensive tests added to not break cases like:

Please check the website (https://nextcloud.com/)
Please check the website https://nextcloud.com/wikipedia_(has-braces)

And the likes. It was a long fight with usability of most users. So we really prefer broken previous over broken UX of normal users posting behaviour

@susnux susnux modified the milestones: 8.21.1, 8.23.0 Jan 15, 2025
@Antreesy Antreesy modified the milestones: 8.23.0, 8.23.2 Feb 13, 2025
@susnux susnux modified the milestones: 8.23.2, 8.24.0 Feb 28, 2025
@ShGKme ShGKme modified the milestones: 8.24.0, 8.25.0 Apr 2, 2025
@susnux susnux modified the milestones: 8.25.0, 8.26.0 Apr 16, 2025
@susnux susnux modified the milestones: 8.26.0, 8.27.0 Apr 27, 2025
@susnux susnux modified the milestones: 8.27.0, 8.27.1 May 28, 2025
@susnux susnux modified the milestones: 8.27.1, 8.28.1 Jul 15, 2025
@susnux susnux modified the milestones: 8.28.1, 8.29.0 Jul 15, 2025
@susnux
Copy link
Contributor

susnux commented Aug 18, 2025

The problem here is also that something like this would wrongly be parsed:
some text https://nextcloud.com#some)

@susnux susnux added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 18, 2025
@susnux susnux marked this pull request as draft August 18, 2025 20:17
@susnux susnux removed this from the 8.29.0 milestone Aug 18, 2025
@provokateurin
Copy link
Author

TBH I don't understand why it would be a problem, since URLs must be encoded and therefore can't contain plain braces.

@susnux
Copy link
Contributor

susnux commented Aug 18, 2025

TBH I don't understand why it would be a problem, since URLs must be encoded and therefore can't contain plain braces.

Depends on the moment of parsing, no? Because if you copy paste an URL from the browser address bar it will contain the ) as is.

@nickvergessen
Copy link
Contributor

TBH I don't understand why it would be a problem, since URLs must be encoded and therefore can't contain plain braces.

Because day-to-day usage shows that they are not pasted encoded, users don't encode and we receive multiple reports in the week after a Talk release when someone changes this 🙈

The problem is even a step further, there are URLs where the brackets are expected (most famous is wikipedia)

  • We are aware of the problem (https://github.com/nextcloud/talk-desktop/issues/1421)
  • We are aware of the problem (already reported in https://github.com/nextcloud/talk-desktop/issues/1421)
  • My city https://en.wikipedia.org/wiki/Stuttgart_(disambiguation)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug Something isn't working feature: richtext Related to the richtext component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parsing differences between NcRichText and Reference API lead to inconsistent link previews
5 participants