[customize-widgets/utils/widgetToBlock] Initialize a widget's raw_content.content to an empty string if it's undefined
#46487
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
fullofcaffeine
force-pushed
the
add/guard-against-undefined-raw-content-in-widget-to-block
branch
from
b98176d to
e8d4587
Compare
dmsnell
reviewed
Dec 13, 2022
|
Size Change: +21 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
fullofcaffeine
added
the
[Type] Bug
dmsnell
approved these changes
Dec 13, 2022
There was a problem hiding this comment.
This approval means "I don't have any further comment or see anything obviously wrong with this."
I'd still recommend finding someone more familiar with the widgets who might be able to speak to why the content was undefined.
If nobody speaks though, maybe we add a comment saying something like It's unclear why this is sometimes undefined, but it shouldn't be. and then merge.
fullofcaffeine
force-pushed
the
add/guard-against-undefined-raw-content-in-widget-to-block
branch
from
2b94966 to
eaa9170
Compare
fullofcaffeine
deleted the
add/guard-against-undefined-raw-content-in-widget-to-block
branch
dmsnell
pushed a commit
that referenced
this pull request
Dec 15, 2022
…content.content` to an empty string if it's `undefined` (#46487) * Initialize a widget's raw_content.content to an empty string if it's undefined * Do not mutate the instance object's raw_instance property * Add comment
|
The attributes within |
noisysocks
added
the
[Feature] Widgets Customizer
@noisysocks Thanks for clarifying! I had no idea bout that. I'll give it another go following your suggestion; how do I check it's a block widget? |
Wrote too fast, sorry. I now see you came up with a proper fix in #46600. Thanks! |
noisysocks
added a commit
that referenced
this pull request
Dec 20, 2022
…'s `raw_content.content` to an empty string if it's `undefined`" (#46600) * Revert "[`customize-widgets/utils/widgetToBlock`] Initialize a widget's `raw_content.content` to an empty string if it's `undefined` (#46487)" This reverts commit 271f650. * Guard against undefined content closer to parse() call
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.
What?
Follow-up to #46475.
We experienced widgets with
raw_content={}, hence it doesn't contain a validcontentproperty. This feeds invalid (undefined) data to the parser and makes it break when it gets to https://github.com/WordPress/gutenberg/blob/trunk/packages/block-serialization-default-parser/src/index.js#L441, with:TypeError: Cannot read properties of undefined (reading 'length').The exception bubbles up and breaks the widget block editor:
After chatting with @dmsnell and with his help (thanks!), we continued troubleshooting the issue, and eventually decided to move the fix up the stack to this function as the root issue is not in the parser. We're still unsure where the actual culprit is as it's been hard to troubleshoot, but a guard in
widgetToBlockseems more viable and safer than modifying the lower-level parser. If anyone has any ideas on where the culprit might be, feel free to comment here.Why?
I've experienced a crash in the customize widgets block editor because the widget JSON object's
rawproperty has a value of{}(empty object). Here's a sample widget object JSON that ended up causing the issue: https://gist.github.com/fullofcaffeine/3807914297e3e836866edf6e49b61963. Notice how therawproperty is{}. We have no idea how it got to this state, and that's why we decided to add the guard in thewidgetToBlockinstead.How?
Add a simple guard that checks if the
raw_contentproperty for the given widget passed over towidgetToBlockhas anundefinedcontentproperty, if so, we set itscontentproperty to''(empty string).Testing Instructions
I have no idea how the widget got to the state where
rawis{}, but you can manually change one of your widgets in your db and set therawproperty to{}in order to reproduce it.Testing Instructions for Keyboard
Screenshots or screencast