Widget Visibility: Restrict to top level blocks

[mreishus]

Fixes #20057

Changes proposed in this Pull Request (TLDR):

• Reduce the scope of the Widget Visibility feature: No longer support recursive filtering and completely remove the render_block() call from filter_widget(). Users are only allowed to set visibility rules on the top level block of each widget.

Changes proposed in this Pull Request:

• In Widget Visibility: add controls to widget block editor #20731, we added "Widget Visibility", an enhancement to gutenberg block widgets which allow users to apply rules that decide when blocks inside widgets display.
• When this was deployed to wpcom, we saw many crashes happening in user configurations that were not happening in a stock configuration. For example, one culprit was wpcom's infinite scroll homepage plugin.
• There is more detail about these crashes here:
  • Widget-Visibility: Out of Memory leads to 500 Error on Several Simple Sites wp-calypso#56526
  • Widget-Visibility: null->render() leads to 500 error on Simple Sites wp-calypso#56546
• In all known cases, crashes were related to filter_widget() running render_block(). Before Widget Visibility was implemented, filter_widget() only returned its argument unchanged or returned false. With Widget Visibility, sometimes it would change the content of the block passed to it by re-rendering the block, and return that.
  • Re-rendering blocks was needed to provide support for nested rules. For example, if a widget was a column block, and the left column had visibility rules XYZ, and the right column had visibility rules ABC, we would rerender the entire column block so we could apply the visibility rules to children of the widget, instead of only the widget itself.
  • filter_widget() running render_block() is risky because so many plugins modify rendering behavior. For example, we saw an infinite loop bug where render_block() would need to seek out sidebar settings, which would end up getting a list of widgets and calling filter_widget(), which would call render_block(), which called filter_widget().. etc.
• I was able to prevent infinite loops by using a static flag on the Jetpack_Widget_Conditions class, but there was at least one render related bug I couldn't fix (wp-calypso#56546).
• My solution is to reduce the scope of the Widget Visibility feature: No longer support recursive filtering and completely remove the render_block() call from filter_widget().
  • Now filter_widget() works the same as before: It only ever returns false or returns its argument unchanged.
  • Scope of potential bugs is drastically reduced.
  • Now users may only set visibility settings on the outermost block of each widget.

Jetpack product discussion

Does this pull request change what data or activity we track or use?

No

Testing instructions:

• jetpack docker up
• Make sure WP is installed and you can visit /wp-admin. You may need to use jetpack docker install, and possibly edit one of the env files such that your password isn't easily guessable if you decide to use a tunneling solution in the future.
• In another terminal, jetpack watch plugins/jetpack
  • You might also need to jetpack build plugins/jetpack first.
• Make sure this setting is turned on in jetpack settings.
  
• Visit http://localhost/wp-admin/widgets.php
• Add a block based widget here
• In the outermost block of any widget, go to the advanced section of the gutenberg sidebar

You should see the following in the sidebar, which lets you set up rules:

When visiting the frontend of the site, those widgets should follow those rules.

• If you select a non-outermost block, you will see a message asking you to select the outermost block. For example, add a columns block, then add a paragraph block inside one of the columns, and attempt to set visibility rules on that paragraph block.

Testing instructions (wpcom)

We need to make a custom patch, since #20057 is merged in JP but reverted on wpcom.

Testing instructions (Unit Tests)

• jetpack docker phpunit -- --filter=Widget_Conditions
  • In order to get the tests working, I had to: jetpack docker sh, then cd /tmp/wordpress-develop/tests/phpunit/data, then svn up -r44722, then cd ../includes/ , then again svn up -r44722. This was done a few weeks ago and I am no longer certain this is working with new docker images.

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ All commits were linted before commit.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

---

Jetpack plugin:

• Next scheduled release: October 5, 2021.
• Scheduled code freeze: September 28, 2021.

[sixhours]

I'm testing this locally, and I'm able to see the widget visibility settings when in /wp-admin/widgets.php, but not in the Customizer for the same widget:

Screen.Recording.2021-09-29.at.10.45.38.AM.mov

[sixhours]

The good news is the widgets behave as expected on my local test site. 👍

[mreishus]

Good catch; fixed.

[samiff]

Before patching, was able to recreate the Fatal error: Uncaught Error: Call to a member function render() on null wp-includes/class-wp-block.php on line 21 by using a column block and setting a mix of nested visibility options.

Then I applied this PR and the fatal was no longer occurring. Was also able to see the Please select the top level block... message on the nested column block elements. Tried some silly nesting but wasn't able to break things.

[jeherve]

@mreishus I've created a WordPress.com diff, D67826-code, to bring us back in sync on WordPress.com.

It tests well for me. I'll let you handle the commit?

Thank you!

[jeherve]

Cherry-picked to jetpack/branch-10.2 in c306f48

[samiff]

r232714-wpcom

Looking good on wpcom so far, keeping an eye on support channels today.