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

Revisions: slash meta values for previews #5344

Closed

Conversation

adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented Sep 28, 2023

Correct an issue where meta values with values like a quote could not be previewed. update_metadata expects data to be slashed.

To test this bug:

  1. publish a post with a footnote
  2. add a quote to the end of the footnote
  3. preview the post

expected: footnote shows quote

actual: footnote is missing or missing quote

Trac ticket: https://core.trac.wordpress.org/ticket/20564

See also: WordPress/gutenberg#52988


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Correct an issue where meta values with values like a quote `”` could not be previewed. update_metadata expects data to be slashed.
@mukeshpanchal27
Copy link
Member

@adamsilverstein, If we remove the wp_slash, the unit tests pass successfully ✅. Do we still need to implement safeguard measures?

Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks @adamsilverstein for the PR. Approved with one question: #5344 (comment)

@adamsilverstein
Copy link
Member Author

@adamsilverstein, If we remove the wp_slash, the unit tests pass successfully ✅. Do we still need to implement safeguard measures?

I added a test that failed before the patch, but removed it to get tests passing in 6927209 because it was causing other tests to fail (I suspect because it wasn't cleaning up completely). I'm going to try to reintroduce it in another spot.

@adamsilverstein
Copy link
Member Author

@mukeshpanchal27 - I reintroduced the test by putting it in the metaRevisions test file and cleaning it up a bit. This test fails before the PR and essentially reproduces the issue surfaced by the GB E2E test.

@adamsilverstein
Copy link
Member Author

Trying again in 413ac9f (#5344); I moved the tests to tests/phpunit/tests/rest-api/rest-autosaves-controller.php and simplified a bit.

@adamsilverstein
Copy link
Member Author

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.

3 participants