backdrop
D7 crossport: Reordering fails with more than 100 items in a menu
This is the respective issue as https://www.drupal.org/project/drupal/issues/1007746, with https://git.drupalcode.org/project/drupal/-/commit/bf9fbe42f38ff9ab043703e8cc22984861893077 being the change that was merged into D7 back in June (which was a backport of what went into D8):
Problem/Motivation
An issue occurs when more than 100 items exist in the root of the menu or under a single parent item, or items exist with a weight outside of -50..50 (causing overflow when trying to fit that weight into the select form element).
Proposed resolution
The main problem is that the delta value of the weight "property" is hardcoded to 50, so if we have more than 100 elements (-50 to 50) we won't be able to sort items in the menu for higher weights.
The solution is to query the database the min and max values from the weight field in the menu_links table, then make the abs value (see detailed explanation at #96).
The last patch pass all tests on all relevant PHP and MySQL releases.
https://www.drupal.org/project/drupal/issues/1007746#comment-5596710
Show row weights is at the heart of the issue. The html select element (dropdown menu) for the row weight is created based on the "delta" value discussed in the thread above. That value is currently hardcoded at 50, so the select element ranges from -50..50. When you have an element with a weight larger than 50, it's an overflow error, and making any modifications to the menu from the editor interface will overwrite the out-of-bounds row weights.
I've updated my patch to include backportability and added test cases (thanks xjm). The new test cases (I just added them to menu.test) try to create a menu of 102 items and assign increasing weights to each element, and fails when trying to set a weight of 51 because the select element caps at 50 (because delta=50). The patched code (in menu.admin.inc and menu.module) removes the hardcoded 50, and adjusts it to match the total number of items already in a menu (or 50 if there are less than 50 items).
RTBC. Looks good! I tested and it works as expected. And the code looks good.
Thanks @herbdool 🙏🏼
...I've rebased the PR branch just in case - no code changes, but we now have failing tests. I'll work on that soon as I get a chance.
Maybe rebase again in case the work on menu in 1.21.1 fixes things. @klonos
Sorry, this has slipped under the radar. I have rebased (no other changes), and the PHP failures are gone, and no PHPCS/cspell complains either.
Setting this back to RTBC, based on @herbdool's previous review.
Thanks @klonos and @herbdool! Sorry this took so long to get reviewed. I merged https://github.com/backdrop/backdrop/pull/3866 into 1.x and 1.25.x.
