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.

Sign up for GitHub

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

Jump to bottom

Late escape Post blocks #37876

Merged
merged 9 commits into from
Jan 13, 2022
Merged

Late escape Post blocks #37876

merged 9 commits into from
Jan 13, 2022

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Jan 11, 2022

This is not a security problem. This PR simply moves escaping of all PHP output to be as "late" as possible. This means we avoid escaping variables until they are output in the HTML markup.

This is a WP Core best practice.

How has this been tested?

  • add both blocks
  • check functionality is "as was"
  • check all tests pass

Screenshots

Types of changes

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 (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@getdave getdave added [Type] Code Quality Issues or PRs that relate to code quality [Block] Post Excerpt Affects the Post Excerpt Block [Block] Post Title Affects the Post Title Block [Block] Post Featured Image Affects the Post Featured Image Block [Block] Post Tags Affects the Post Tags Block [Type] Security Related to security concerns or efforts [Block] Post Navigation Link Affects the Post Navigation Link Block [Block] Post Template Affects the Post Template Block labels Jan 11, 2022
@getdave getdave requested a review from ajitbohra as a code owner January 11, 2022 13:54
@getdave getdave self-assigned this Jan 11, 2022
@getdave getdave requested a review from aristath January 12, 2022 12:33
@getdave
Copy link
Contributor Author

getdave commented Jan 13, 2022

Thank you for your patience @aristath 🙇

@getdave
Remove redunant escaping
621ed38 
`get_block_wrapper_attributes` will escape for us. This is as per feedback on other PRs involving late escaping.
@getdave getdave merged commit f0982d1 into trunk Jan 13, 2022
@getdave getdave deleted the update/late-escape-post-blocks branch January 13, 2022 14:06
@github-actions github-actions bot added this to the Gutenberg 12.5 milestone Jan 13, 2022
@scruffian
Copy link
Contributor

This creates a problem when the post date is a link:
Screenshot 2022-01-14 at 17 14 46

@getdave
Copy link
Contributor Author

getdave commented Jan 17, 2022

@scruffian Let me fix that now.

@getdave getdave mentioned this pull request Jan 17, 2022
Merged
8 tasks
@getdave getdave mentioned this pull request Feb 7, 2022
Closed
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aristath aristath aristath approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

Assignees

@getdave getdave

Labels
[Block] Post Excerpt Affects the Post Excerpt Block [Block] Post Featured Image Affects the Post Featured Image Block [Block] Post Navigation Link Affects the Post Navigation Link Block [Block] Post Tags Affects the Post Tags Block [Block] Post Template Affects the Post Template Block [Block] Post Title Affects the Post Title Block [Type] Code Quality Issues or PRs that relate to code quality [Type] Security Related to security concerns or efforts
Projects
None yet
Milestone
Gutenberg 12.5
Development

Successfully merging this pull request may close these issues.
3 participants
@getdave @scruffian @aristath