Protect against recursive customizer navigation menu data

[donpark]

Recursive data can slip into navigation menu structure but client-script currently has no protection against, resulting in browser freezing when trying to edit navigation menus.

Attached patch changes getDepth() to keep track of parent navigation menu item ID to avoid recursion.

[dlh]

Hey @donpark — thanks for submitting this report and patch. Are you able to provide steps to replicate the bug in core?

[donpark]

Replying to dlh:

Hey @donpark — thanks for submitting this report and patch. Are you able to provide steps to replicate the bug in core?

Only in context of a particular site on WordPress.com. And testing plan, detailed in the A8C internal Phabricator patch I mentioned (D29638-code), requires the tester be an Automattician.

Given these obstacles, I think this ticket is best assigned to an Automattician with WP.org commit privileage.

• Don

[dlh]

OK. Speaking for myself, there's nothing I can do to "assign" the ticket to anyone. So, absent more information, I'm not sure what to do other than wait for an Automattician to volunteer to try to confirm that there's a bug in core and, if so, how to replicate it. (To that end, I'll leave the reporter-feedback tag on the ticket for now.)

[donpark]

Replying to dlh:

OK. Speaking for myself, there's nothing I can do to "assign" the ticket to anyone. So, absent more information, I'm not sure what to do other than wait for an Automattician to volunteer to try to confirm that there's a bug in core and, if so, how to replicate it. (To that end, I'll leave the reporter-feedback tag on the ticket for now.)

Fair enough. I'll see if I can get someone here to pick up the baton. Thx for the help, David. :-)

[celloexpressions]

fix_customizer_nav_menu_freeze.diff​ seems reasonable to me but I haven't dug into this part of the code for several years. Since it sounds like this surfaced as an issue on WP.com, it would be great if someone from Automattic could own testing and committing this patch.

[SirLouen]

Replying to donpark:

Only in context of a particular site on WordPress.com. And testing plan, detailed in the A8C internal Phabricator patch I mentioned (D29638-code), requires the tester be an Automattician.

If the tester requires being an "Automattician" as you say, this is not the place to report this. There are some ​WordPress.com forums specifically for these types of issues. If anyone from wordpress.com happens to isolate the problem, it can be brought here for further testing by any contributor.

This post should be left on hold until you have detailed instructions on how to reproduce this by anyone, anywhere, anywhen. Here are some ideas on ​how to build a Testing Use Case with, maybe, a minimal plugin that reproduces the problem (and your patch happens to solve it)