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 content attribute selector #178

Merged
merged 3 commits into from
Dec 18, 2020
Merged

Fix content attribute selector #178

merged 3 commits into from
Dec 18, 2020

Conversation

yscik
Copy link
Contributor

@yscik yscik commented Nov 17, 2020

Follow-up to #163

Changes proposed in this Pull Request

  • Change back the selector for content attribute, since the extra <code> wrapper from Fix escaping issues with HTML entities #160 was not needed after all.
    • This also caused an issue of the block being re-created from the deprecated helper on every open
  • This also removes the need for the deprecated helper — this would make sure block output from older versions are compatible, but the block output is not going to change in 3.5.6

Testing instructions

  • Create a code block in master
  • Switch to this branch
  • Edit the post and ensure the block works without any warnings
  • Change/add new code blocks, save post.
  • Reload the block editor and check that block still works without warning.

@yscik yscik added this to the v3.5.6 milestone Nov 17, 2020
@yscik yscik requested a review from jom November 17, 2020 15:33
@jom
Copy link
Member

jom commented Nov 17, 2020

Something strange happens with this when coming from 3.5.5.

I created a block with the test content from #152. I then switched to this branch, edited the page, saved it, and escaping seems to have been lost.

Screen Shot 2020-11-17 at 4 50 18 pm

This didn't happen in the page I created on master. Is this expected with how content escaping is changing?

@yscik
Copy link
Contributor Author

yscik commented Nov 18, 2020

The fix in #160 is implemented when saving the block, so it won't work with existing content from 3.5.5 — it will still break the content when loading the non-escaped output of a 3.5.5 block. When the post content is parsed, both &cent; and ¢ will be read as ¢.

I tried an approach of escaping the block content on the REST API before it hits the editor, which could also fix this issue for existing content. Even though the block would pick up the correct content, block validation then fails because the post content (escaped) is not the same as the save output (not escaped) 😫 . This would likely need setting up a block version attribute and conditionally escaping, but it's a pretty fragile solution already.

Considering that these string in existing content are likely already broken if it was edited even once, it might not be worth it to complicate this further. cc @alexsanford any opinions about this?

Copy link
Member

@jom jom left a comment

Choose a reason for hiding this comment

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

Needs a rebase to work with the refactor, but I'm happy with this approach 😃

@yscik yscik merged commit 5fe463a into master Dec 18, 2020
@yscik yscik deleted the fix/attribute-selector branch December 18, 2020 14:32
@alexsanford alexsanford modified the milestones: v3.5.6, v3.6.0 Feb 12, 2021
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.

None yet

3 participants