Skip to content

#65414 Refactor wp_ajax_delete_inactive_widgets(), extract wp_delete_inactiv… - #12965

Closed
PANawkar wants to merge 1 commit into
WordPress:trunkfrom
PANawkar:65414-local
Closed

#65414 Refactor wp_ajax_delete_inactive_widgets(), extract wp_delete_inactiv…#12965
PANawkar wants to merge 1 commit into
WordPress:trunkfrom
PANawkar:65414-local

Conversation

@PANawkar

Copy link
Copy Markdown

Extracts the widget-removal logic from wp_ajax_delete_inactive_widgets() into a new reusable wp_delete_inactive_widgets() function in wp-admin/includes/widgets.php, per the existing refactor in this PR.

Additionally addresses the outstanding review comment: guards against a PHP 8 TypeError by checking is_array( $inactive_widgets ) before calling count(), since $sidebars_widgets['wp_inactive_widgets'] isn't guaranteed to be an array if the option data is corrupted.

Also adds PHPUnit coverage for wp_delete_inactive_widgets() in tests/phpunit/tests/widgets.php, covering:

  • normal removal of inactive widget settings and emptying of the sidebar
  • a missing wp_inactive_widgets key (no-op)
  • an empty wp_inactive_widgets array (no-op)
  • a non-array wp_inactive_widgets value, verifying no TypeError is thrown (regression test for the PHP 8 fix above)
  • a corrupted, non-array widget_* option being left untouched
  • Trac ticket: https://core.trac.wordpress.org/ticket/65414

@github-actions

Copy link
Copy Markdown

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: @pratik.nawkar@xecurify.com.

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

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

@t-hamano

Copy link
Copy Markdown
Contributor

Thanks for the PR. However, since a review is already in progress for an existing PR, there is no need to submit the same PR again.

@t-hamano t-hamano closed this Aug 11, 2026
@PANawkar

Copy link
Copy Markdown
Author

Hi @t-hamano, thanks for taking a look, and apologies for the confusion - I didn't mean to duplicate the effort on #12099.

I opened this to help pick up on your outstanding review comment there, and also added PHPUnit test coverage for wp_delete_inactive_widgets(), since the original PR didn't have tests yet.

Since I don't have push access to @pbearne's branch, I wasn't sure how else to contribute the fix + tests, so I opened this separately - happy to be corrected if there's a better way to do this. Would you be okay reopening this so it can be reviewed, or would you prefer I add these as review comments directly on #12099 instead and let @pbearne fold them in? Either way works for me - just want to help get this one over the line.

Thanks again for the thorough review!

@t-hamano

Copy link
Copy Markdown
Contributor

@PANawkar Thanks for your contribution!

I opened this to help pick up on your outstanding review comment there, and also added PHPUnit test coverage for wp_delete_inactive_widgets(), since the original PR didn't have tests yet.

This is appreciated, but let's avoid it as a general rule, as it can be confusing for other contributors to decide which PR to review. Unless an existing PR is very old or requires urgent attention.

For now, I think it's sufficient to leave #12099 as is and just wait for a reply from @pbearne.

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.

2 participants