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

Fix encoded characters in navigation link text #30182

Closed
wants to merge 2 commits into from
Closed

Fix encoded characters in navigation link text #30182

wants to merge 2 commits into from

Conversation

donnapep
Copy link
Contributor

@donnapep donnapep commented Mar 24, 2021

Description

Fixes #30111. Renders title.raw instead of title.rendered.

How has this been tested?

  1. Add the navigation block.
  2. Add a new menu item.
  3. Select adding a new Link and begin typing the title of a page that doesn't exist. When you do so, include &.
  4. Select the option to create a page draft.
  5. See that the menu item shows the ampersand.
  6. Select adding a new Post Link and begin typing the title of a page that doesn't exist. When you do so, include &.
  7. Select the option to create a post draft.

Screenshots

Screen Shot 2021-03-24 at 6 59 13 AM

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@donnapep donnapep requested a review from ajitbohra as a code owner March 24, 2021 11:00
@annezazu annezazu added [Block] Navigation Affects the Navigation Block [Block] Post Navigation Link Affects the Post Navigation Link Block [Type] Bug An existing feature does not function as intended labels Aug 25, 2021
@gwwar
Copy link
Contributor

gwwar commented Sep 22, 2021

Whoops, looks like this one got lost in the PR list. The code change here looks correct. @donnapep do you mind rebasing with trunk (or let us know if you'd like us to take over). I can help review this one

@carolinan carolinan added [Block] Navigation Link Affects the Navigation Link Block and removed [Block] Post Navigation Link Affects the Post Navigation Link Block labels Nov 2, 2021
@annezazu
Copy link
Contributor

annezazu commented Feb 7, 2022

Seems like this one should likely be taken over :)

@Mamaduka Mamaduka requested a review from getdave April 6, 2022 19:02
@getdave getdave self-assigned this Apr 29, 2022
Copy link
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

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

Thank you for raising this and apologies that it flew under my radar.

I've tested and I think it might be more complex than I'd hoped. Especially as it no longer appears to work.

Screen.Capture.on.2022-05-13.at.20-41-44.mov

I think this is all happened because we're escaping the title/label prematurely (i.e. before the point at which it is rendered).

The escaping of the label within the updateNavigationLinkBlockAttributes method which causes the post.title.rendered to be converted into an &. This still happens with .raw as shown in the video above.

Normal Post links are rendered into a <RichText> so the escaped HTML renders correctly. However, draft pages (those created inline within the Link UI) are rendered into a <span> which means that the escaped HTML shows up in it's raw form.

I will raise a PR which properly applies the escaping at the point of output which will allow us to be more selective in our application of escaping.

@getdave
Copy link
Contributor

getdave commented Jun 13, 2022

This was fixed here #41063

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Link Affects the Navigation Link Block [Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation Block: Creating a draft page with “&” results in HTML Entities
5 participants