Layout selection weight sorting

This should work.

The FAIL patch has the expected failure.

For the PASS patch, I forgot that grouped definitions are also sorted, had to fix that test case too.

Adding credit

I am having issues applying this to 8.7.7

Through breakpoints I can confirm that `getSortedDefinitions` never seems to get called and so the sorting doesn't apply.

Assuming there's some update in 8.8 and 8.9 that call this but it's missing in 8.7.7?

Well, none of that _actually_ worked because we don't sort our plugins when displaying them in the UI. Which is a bug on its own.
Here's a fix, but this needs a UI test.

Fixing existing tests. Still needs dedicated test coverage.

The interactions between getFilteredDefinitions(), getSortedDefinitions(), and getGroupedDefinitions() is odd.

getGroupedDefinitions() always calls getSortedDefinitions()

Of the five calls to getFilteredDefinitions():
2 are followed by calls to getGroupedDefinitions()
1 is followed by a call to getSortedDefinitions()
2 are left unsorted (the case in ChooseSectionController that affects this patch, and in BlockForm when presenting the visibility conditions)

Which means that if hook_plugin_filter_TYPE_alter()/hook_plugin_filter_TYPE__CONSUMER_alter() are used to sort plugins, in 3 of the 5 cases it will be undone by the code that follows. Which is unfortunate.

I see two ways forward.
One is to embrace the pattern of other calls to getFilteredDefinitions() and add a sort after the call.
Another is to add getSortedFilteredDefinitions() and getGroupedFilteredDefinitions() methods, somehow.

And even if we want to do the 2nd approach, I think we can do that after the first approach in a follow-up.

Still needs tests.

NW for tests

Reroll for 9.3.x.

Just ran into this issue here: #3088073: Use weight to sort the layout selection

Weight is essentially needed and we should add it. I'll add it as TODO and hope we can finally finish this one ;)

I manually applied patch "3089418-22.patch" on the issue branch.

All done! I hope the provided test changes suffice! Failing tests seem to be unrelated! See #3315678: strnatcasecmp(): Passing null to parameter #2 ($string2) of type string is deprecated, for a similar issue.

EDIT: Still unsure, why the tests are not failing on 11.x-dev... They are caused by line 26-209 of LayoutPluginManager:

      if ($a->getCategory() != $b->getCategory()) {
        return strnatcasecmp($a->getCategory(), $b->getCategory());
      }
      return strnatcasecmp($a->getLabel(), $b->getLabel());

But yea, as we have not touched that part, I still think it is unrelated.

Never mind, there are also failing tests related to this issue.

Alright, now it should be fixed.

@Grevil: Still 6 failing tests here.

As mentioned in #32, they seem unrelated.

Have not yet reviewed or tetsed

But seems to have some test failures.

Usually there's just 1 random failure but 6 consistently means the MR is causing them. If they're fixed in #3315678: strnatcasecmp(): Passing null to parameter #2 ($string2) of type string is deprecated then this should be postponed as it can't be merged with failures.

Thank you, @smustgrave! I'll take a deeper look later on. The test failures are also very consistent, even after rebasing, so it probably is somehow related to the changes here.

@smustgrave #37 references an unrelated issue with the same error. I just ran into exactly this issue in LayoutPluginManager.
We need a separate, but similar issue as referenced in #37 but for LayoutPluginManager!

Here are other similar issues:

• #3280848: Shortcut links without a title cause deprecation notices
• #3306453: Resolve Deprecated function: strnatcasecmp(): Passing null to parameter #2

Here's the correct issue: #3392572: Add missing category to Drupal\layout_builder\Plugin\Layout\BlankLayout and let modules and themes alter the list of layouts

Please review #3392572: Add missing category to Drupal\layout_builder\Plugin\Layout\BlankLayout and let modules and themes alter the list of layouts to get it merged asap so we can proceed here :)