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

V14/bugfix/html link parser regex #17466

Closed

Conversation

Dark0d3
Copy link

@Dark0d3 Dark0d3 commented Nov 8, 2024

LocalLinkTagPattern regex was failing and updated to the latest one the same as v15.

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes: issue

Description

Added a new regex pattern (LocalLinkPatternNew) in HtmlLocalLinkParser, LocalLinkPatternNew is the same as v15.

The previous regex pattern (LocalLinkTagPattern) was failing inside RTE as mentioned in the issue above or the forum

Issue in hindsight is that the failing of regex results in the rendering of links in RTE as something like this
href="/{localLink:dff7c2bc-4fb8-4fd1-af78-c826124cf0dc}"

<p><a rel="noopener" type="document" href="/{localLink:dff7c2bc-4fb8-4fd1-af78-c826124cf0dc}" target="_blank" title="dasdasdsa">dasdasdsa&nbsp; &nbsp;d</a></p>
<p>&nbsp;</p>
<p><a rel="noopener" type="document" href="/{localLink:dff7c2bc-4fb8-4fd1-af78-c826124cf0dc}" target="_blank" title="test">test</a></p>
  1. Testing the current regex LocalLinkTagPattern with the above text
  2. Do the same with the LocalLinkPatternNew regex

LocalLinkTagPattern regex was failing and updated to the latest one the same as v15.
Copy link

github-actions bot commented Nov 8, 2024

Hi there @Dark0d3, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

  • It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
  • The automated tests all pass (see "Checks" tab on this PR)
  • The level of security for this contribution is the same or improved
  • The level of performance for this contribution is the same or improved
  • Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
  • If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
  • 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@emmagarland
Copy link
Contributor

Hi @Dark0d3 thanks for this! I see you have a conversation with @Migaroez about this one on the issue. Before anyone in the core collabs team or HQ can resolve this one, I just wanted to flag that some merge conflicts have appeared in this PR, in case you hadn't seen.

Thanks so much for your contribution and hopefully this one can help us to apply the fix to 14 too.

Emma

@Dark0d3 Dark0d3 changed the base branch from contrib to v14/dev November 9, 2024 08:41
@Dark0d3
Copy link
Author

Dark0d3 commented Nov 9, 2024

Hi @emmagarland, that was my bad I was trying to merge to the wrong branch. I have changed it now.

@Migaroez
Copy link
Contributor

I am going to close this PR as #17288, which is a more complete solution, has already been cherry picked into v14 1581eb6 and will be part of the next v14 release.

@Migaroez Migaroez closed this Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

V14 Html local link parser regex fails (RTE)
3 participants