Block Editor: Fix Block editor crash #56051
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
t-hamano
added
[Type] Bug
ellatrix
approved these changes
Nov 11, 2023
|
Thank you for your quick review! |
t-hamano
changed the title
|
Size Change: +3 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
@ellatrix The editor crashes on trunk, so it seems like the performance test just won't pass. Can we force merge? |
|
What's the problem? |
|
The two actions that are failing are showing the following errors: I think this error is occurring in this test file. This test actually tries to insert the block, but the block editor is broken in trunk, so I'm expecting the test to always fail. |
|
Forced it |
|
Thankfully, all GitHub Action tests on trunk seem to have passed. |
cbravobernal
pushed a commit
that referenced
this pull request
Nov 14, 2023
jeryj
reviewed
Nov 14, 2023
| blockEditingMode, | ||
| hasParents, | ||
| showParentSelector, | ||
| selectedBlockClientId, |
There was a problem hiding this comment.
Thanks for catching this! It doesn't look like selectedBlockClientId is used at all. Does it work to remove selectedBlockClientId from line 48 instead?
There was a problem hiding this comment.
Thanks for the reply. Isn't selectedBlockClientId needed here?
There was a problem hiding this comment.
Ah! I was still checked out on the add/top-toolbar-slot branch #55787. In that branch it wasn't used and so the tests weren't failing. Once #56008 was also merged to trunk, it became an issue and your fix is the right one. Sorry for the confusion, and thank you for the fix! I feel better now understanding why the bug happened.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #55787
What?
This PR fixes a crash when clicking on the editor canvas.
d1eb3d42659d6355fd92e417ed6df6e9.mp4
Why?
The following is logged in the browser console:
It looks like the
selectedBlockClientIdobtained via useSelect is not defined correctly as a variable.How?
See diff
Testing Instructions
It should pass all tests correctly.