Skip to content

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

Closed
dkotter wants to merge 1 commit intodevelopfrom
fix/block-crash
Closed

Ensure the attributes.content param exists before using it#220
dkotter wants to merge 1 commit intodevelopfrom
fix/block-crash

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 Sidsector9 and iamdharmesh 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.

1 participant