Revert "[customize-widgets/utils/widgetToBlock] Initialize a widget's raw_content.content to an empty string if it's undefined"
#46600
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
|
Size Change: -5 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
dmsnell
reviewed
Dec 16, 2022
| if ( idBase === 'block' ) { | ||
| const parsedBlocks = parse( raw.content, { | ||
| const parsedBlocks = parse( raw.content ?? '', { |
There was a problem hiding this comment.
if we're going to revert this change because the block shouldn't have content, maybe we add the check in the if and avoid calling parse() when we know it won't contain anything?
I still feel like this is also an okay workaround, but the implications aren't clear, nor is the reason why some have content and other don't.
the idBase === 'block' must be the culprit? by assuming the presence of this content property when it shouldn't make that assumption.
There was a problem hiding this comment.
if we're going to revert this change because the block shouldn't have
content, maybe we add the check in theifand avoid callingparse()when we know it won't contain anything?
That would result in this function creating a core/legacy-widget block for a block widget which will display an empty textarea, probably alongside some PHP notices because content is undefined.
At least with this approach we end up with an empty paragraph block that is useable.
I still feel like this is also an okay workaround, but the implications aren't clear, nor is the reason why some have content and other don't.
Indeed. Where did this come up? Was it a user bug report? @fullofcaffeine shared the API response but it doesn't explain how the data ended up that way. Seems to me there's invalid data in the database. The proper fix is to prevent that from happening.
There was a problem hiding this comment.
A nuance of widgets in WordPress is that you can add a widget to a sidebar without ever submitting the widget's form (and therefore calling WP_Widget::update()) so perhaps something to check is that maybe this bug came from:
- Activate Classic Widgets plugin and a non-block theme.
- Go to Appearance → Widgets.
- Drag Block widget to a sidebar. Don't press save or do anything else.
- De-activate Classic Widgets plugin and go to Appearance → Customize.
If that's the case then we need to update all parts of the codebase (PHP and JS) to operate under the assumption that idBase = 'text' implies instance.raw: { content?: string } instead of the current assumption that idBase = 'text' implies instance.raw: { content: string }.
There was a problem hiding this comment.
Was it a user bug report? @fullofcaffeine shared the API response but it doesn't explain how the data ended up that way. Seems to me there's invalid data in the database. The proper fix is to prevent that from happening.
Yes, it was originally reported here: https://wordpress.com/forums/topic/sidebar-widget-error/. Sorry for not adding a link to it in the original PR! Unfortunately, we don't know how the content data got into this state :(
Thanks for working on it! I'll test that shortly, but the fix makes sense and looks like it'd work well! |
6c7c64e to
87c25b7
Compare
|
Thanks @fullofcaffeine, let me know when you've tested. |
87c25b7 to
16aa301
Compare
16aa301 to
d561493
Compare
|
Performed some extra testing on this and confirmed that the widgets customiser still works—no noticeable regressions. I'm going to merge it now before the release is cut so that we're not shipping #46487 in 14.9. |
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.
Reverts #46487
See #46487 (comment).
Guarding against an unexpectedly
undefinedinstance.raw.contentbefore sending it toparse()is okay but #46487 setsinstance.raw.content = ''on all widget types, not just block widgets.Does this simpler fix that only affects block widgets fix the issue for you @fullofcaffeine?