Fix migration of localized D6 menu links

@quietone, I've spent some time reviewing this one, though I'm starting to feel a little overmatched. Here's where I am...

First, I confirmed the bug doing a migration from the d6 fixture through the UI. After applying the patch and doing it all over, the menu link item was migrated correctly.

Here's the menu item in D6

Migrated into D9.4 without the patch

Migrated into D9.4 with the patch

So manual testing was good. Now for the code.

1. I'm a little confused by the naming of the new source properties skip_localized and not_localized. From an English standpoint, they look equivalent to me. We would skip localized migration if it is not localized.

2. Is there a reason that we are using two properties for this? Since they are always opposite, would we be able to use a single property? Or could we even just rely on the langcode property?

3. I'm struggling to parse the tests, probably in part because I'm just not that strong in translation and localization. (I don't even know if those are different things!) But I think something that would help me would be if the test-only patch would simply demonstrate the existing bug. Right now it fails with this message:

No migrations created for id 'd6_menu_links_localized'.

Not a surprise that migration doesn't exist since it's being added in the next patch! Do you think you could come up with something that just shows d6_menu_links failing? It looks like the existing tests were trying to assert translated menu links. Why were those assertions not catching this problem?

@quietone, thank you for the updated fail test. That totally cleared this up for me. The existing assertions are testing the langcode attribute on the link, which is not the same as the langcode of the entity. That is the biggest thing I wasn't putting together.

And I think I get the localization/translation difference here. The menu link we are looking at only exists in French. It's not an English entity that also has a French translation. Is that right?

I like the `is_localized` flag a lot. But I wonder if rather than setting it to `FALSE` we could just keep it `NULL`.

+++ b/core/modules/menu_link_content/src/Plugin/migrate/source/MenuLink.php
@@ -141,6 +141,20 @@ public function prepareRow(Row $row) {
+    $row->setSourceProperty('is_localized', FALSE);

And instead of `is_bool` we could just use `empty` as the callable, which is a more common function and reads more easily to me.

process:
  skip_localized:
    -
      plugin: callback
      callable: empty
      source: is_localized
    -
      plugin: skip_on_empty
      method: row

I think we could do that even if you prefer setting `is_localized` to `FALSE`.

@quietone, right you are! It looks like is_null is callable. What do you think about relying on NULL and is_null instead of FALSE and is_bool?

Thanks, @quietone! Sorry for putting you through the wringer. I know the tests are still running but there's no reason they should fail. I'm going to RTBC this, and hopefully I won't get embarrassed by a failed test.

Random fails in FunctionalJavascript tests. Back to RTBC.

More random fails in FunctionalJavascript tests. Back to RTBC.

29,412 pass, 64 fail

Holy moly! This can't have been related to this patch. Back to RTBC.

Committed and pushed 53783642d7 to 10.0.x and ef28407352 to 9.5.x and a92ee9af4e to 9.4.x. Thanks!