Skip to content

Widget Screen: Problem with updating widgets #69704

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.

Sign up for GitHub

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

Jump to bottom
Open
snehapatil2001 wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open

Widget Screen: Problem with updating widgets #69704

snehapatil2001 wants to merge 6 commits into from
+20 −1

Conversation

@snehapatil2001
Copy link
Contributor

@snehapatil2001 snehapatil2001 commented Mar 25, 2025 •
edited by Mamaduka
Loading

What?

Closes #69702.
Fixes an issue where pressing Enter within a paragraph block in the Widgets Editor would cause the save operation to hang indefinitely. The problem occurred when splitting paragraphs by adding line breaks between words.

Why?

The root cause was duplicate __internalWidgetId values being maintained when a paragraph splits:

  1. Both original and new paragraph kept same internal widget ID
  2. Caused silent save failures
  3. UI showed "Saving..." indefinitely

How?

  1. Resets __internalWidgetId for paragraph blocks before saving
  2. Creates processedBlocks to maintain clean data flow
  3. Uses updatedPost for consistency
  4. Maintains all existing functionality

Testing Instructions

  1. Go to Appearance → Widgets
  2. Add paragraph block and type text
  3. Press Enter between words to split
  4. Click "Update"
  5. Verify:
    • Save completes successfully
    • Both paragraphs remain intact
    • No hanging "Saving..." state

Screenshots or screencast

Screen.Recording.2025-03-26.at.3.54.43.AM.mov

@github-actions
Copy link

github-actions bot commented Mar 25, 2025 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @JelleTempelman.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: JelleTempelman.

Co-authored-by: snehapatil2001 <snehapatil02@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: SirLouen <sirlouen@git.wordpress.org>
Co-authored-by: Mayank-Tripathi32 <mayanktripathi32@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@SirLouen SirLouen mentioned this pull request Mar 26, 2025
Open
6 tasks
@SirLouen
Copy link
Member

SirLouen commented Mar 26, 2025

Tested this patch:
✅Solves the problem
Full Test Report

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Feature] Widgets Screen The block-based screen that replaced widgets.php. [Package] Edit Widgets /packages/edit-widgets labels Mar 26, 2025
@Mamaduka
Copy link
Member

Mamaduka commented Mar 26, 2025

Continuing discussion from the issue. It seems odd that the issue only affects the paragraph block, but no other blocks have "splitting" support enabled.

We should avoid hardcoded special cases for blocks as much as possible. I think there could be a different root cause of the problem that we might be overlooking here.

@Mamaduka Mamaduka changed the title Problem with updating widgets. Widget Screen: Problem with updating widgets Mar 26, 2025
@snehapatil2001
Copy link
Contributor Author

snehapatil2001 commented Mar 26, 2025 •
edited
Loading

@Mamaduka Thanks for the feedback! Based on my analysis and the changes made.
#69702

Buttons/Lists

  • Behavior: Splitting creates structural children (core/button, list-item).
  • Key Difference: These children intentionally skip widget IDs.

Headings

  • Behavior: Splitting converts headings to paragraphs (different block type).
  • Key Difference: New block type triggers fresh ID generation.

Legacy Widgets

  • Behavior: Use id_base + instance instead of __internalWidgetId.

Why Paragraphs Break

  • Consequence: Both split halves share the same ID → Save fails due to duplicate widget entries.

Changes Made

  • Previously, we only reset __internalWidgetId for Paragraph blocks to prevent conflicts.
  • Now, we clear __internalWidgetId for all non-legacy widgets, ensuring unique widget IDs and avoiding save issues.
  • Legacy widgets are excluded, as they manage their own IDs separately.

This approach avoids hardcoded block-specific handling while addressing the root cause of the issue. Let me know if you have any thoughts!

Mamaduka
Mamaduka requested changes Mar 26, 2025
View reviewed changes
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, @snehapatil2001!

Unfortunately, I don't think that modifying the block-editor package is the right approach here (see the inline comment). It's been a while since I worked on Widget packages, so it's hard for me to suggest a better alternative 😞

I know that the previous solution of removing __internalWidgetId worked, but entirely removing attributes before saving might have other consequences, which need to be adequately tested.

Related #29693.

packages/block-editor/src/store/actions.js Outdated
Comment on lines 1008 to 1011
// Add widget ID clearance for same-block splits
...( blockA.clientId === blockB.clientId && {
__internalWidgetId: undefined,
} ),
Copy link
Member

@Mamaduka Mamaduka Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think adding a special case to the block-editor package is a good option. Why?

  • The block-editor doesn't need to know about Widget editor specifics. The __internalWidgetId is an internal ID for that screen.
  • What happens when we have more unique attributes like __internalWidgetId? Do we keep maintaining the list?

Copy link
Contributor Author

@snehapatil2001 snehapatil2001 Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mamaduka Understood—I'll revert the block-editor changes. Please let me know if any other changes are required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mamaduka Mamaduka Mamaduka requested changes

@draganescu draganescu Awaiting requested review from draganescu draganescu is a code owner

@adamziel adamziel Awaiting requested review from adamziel adamziel is a code owner

@kevin940726 kevin940726 Awaiting requested review from kevin940726 kevin940726 is a code owner

@ellatrix ellatrix Awaiting requested review from ellatrix

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

[Feature] Widgets Screen The block-based screen that replaced widgets.php. [Package] Edit Widgets /packages/edit-widgets [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Problem with updating widgets

4 participants

@snehapatil2001 @SirLouen @Mamaduka @t-hamano