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

Ensure the attributes.content param exists before using it #220

Closed
wants to merge 1 commit into from

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Oct 17, 2023

Description of the Change

As described on WordPress.org, with the latest update, some blocks are crashing. I installed the plugin mentioned (Spectra Blocks) and when using the blocks that plugin adds (like Call to Action) I was able to reproduce the reported issue.

The problem stems from the work done in #207, in particular this line: https://github.com/10up/insert-special-characters/pull/207/files#diff-bfe9874d239014961b1ae4e89875a6155667db834a410aaaa2ebe3cf89820556R117. It seems some blocks don't have the attributes.content set, so when we try and access that, it throws a JS error and prevents the block from working.

I've fixed that in this PR but ensuring that data exists before we try to use it and that does seem to fix the reported issue. I'm not sure if it's the best approach though or if there's a better way to handle this. In addition, while this prevents the block from breaking, the toolbar is still shown but doesn't seem to work. Not sure if there's a way to make the toolbar work in this instance or if there's a way to remove the toolbar all together? Not sure if this is an instance where we could be doing something different or if it's an issue with how these custom blocks are built, since they don't seem to have the standard content attribute.

How to test the Change

  1. Install the latest released version of this plugin
  2. Install the Spectra Blocks plugin
  3. Create a new post and add the Call to Action block. Note that the block throws an error
  4. Add the changes from this PR and test again. The Call to Action block should work now
  5. Ensure core blocks (like Paragraphs and Headings) still show the toolbar and inserting special characters work

Changelog Entry

Fixed - Ensure custom blocks don't break.

Credits

Props @dkotter

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@dkotter dkotter self-assigned this Oct 17, 2023
@dkotter dkotter requested a review from a team as a code owner October 17, 2023 14:53
@dkotter dkotter requested review from iamdharmesh and Sidsector9 and removed request for a team and iamdharmesh October 17, 2023 14:53
@dkotter dkotter added this to the 1.1.1 milestone Oct 17, 2023
@dkotter
Copy link
Collaborator Author

dkotter commented Oct 17, 2023

@Sidsector9 Tagging you in for a review here since you worked on the changes in #207. The changes here seem to fix the reported issue but not sure if this is the best approach or not (read through the PR description for full details on that). Thanks!

@dkotter
Copy link
Collaborator Author

dkotter commented Oct 17, 2023

@Sidsector9 As a quick follow up here, I tested the previously released version (v1.0.7) with Spectra Blocks and you can insert special characters within their blocks. So while this PR fixes the immediate issue of crashes, there's still a separate regression where special characters aren't inserted properly, which I imagine probably impacts other blocks.

@dkotter
Copy link
Collaborator Author

dkotter commented Oct 17, 2023

In talking with @jeffpaul, for the sake of getting a full fix out as soon as possible, going to revert #207 (see #221) and we can come back to that PR to see if we can get it working properly. As such, this PR is no longer needed.

@dkotter dkotter closed this Oct 17, 2023
@dkotter dkotter removed this from the 1.1.1 milestone Oct 17, 2023
@dkotter dkotter deleted the fix/block-crash branch October 17, 2023 16:41
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

1 participant