Skip to content
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.

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

Remove the __unstableLocation attribute prior to rendering #33175

Closed

Conversation

pbking
Copy link
Contributor

@pbking pbking commented Jul 2, 2021

Description

Remove the __unstableLocation attribute before passing the attributes to
the menu renderer to avoid an infinate loop if no menu is assigned to
the location.

How has this been tested?

Activate a theme that uses the Navigation block and an __unstableLocation attribute. Don't assign a menu to that location and attempt to render the site (any page that renders the menu block).

Note that before the fix WordPress runs out of memory (assuming due to an infinite loop).

With the fix the page renders as expected.

Types of changes

A simple bugfix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

Remove the __unstableLocation attribute before passing the attributes to
the menu renderer to avoid an infinate loop if no menu is assigned to
the location.
@pbking pbking requested a review from ajitbohra as a code owner July 2, 2021 15:32
@MaggieCabrera MaggieCabrera added [Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended labels Jul 5, 2021
$location = $attributes['__unstableLocation'];
$maybe_classic_navigation = gutenberg_render_menu_from_location( $location, $attributes );
$location = $attributes['__unstableLocation'];
$attributes['__unstableLocation'] = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

could this be unset( $attributes['__unstableLocation'] )?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, that works better, thank you.
I am including that change in this PR.

@pbking
Copy link
Contributor Author

pbking commented Jul 6, 2021

This fix is included in this PR which is moving this effected logic to another location. If that PR gains traction this one should be closed.

@Mamaduka
Copy link
Member

Thanks for working on this, @pbking.

I'm going to close this one since #33244 already got merged.

@Mamaduka Mamaduka closed this Jul 21, 2021
@scruffian scruffian deleted the fix/unstable_location_infinate_loop branch July 21, 2021 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants