Some block categories are not translatable

@dschenk and me are working on this, while sprinting at DrupalCon in Barcelona.

In this patch we added the category explicitly to the corresponding blocks in the system module, to make them translatable.

I helped in the creation of the patch.

I was able to reproduce the issue and confirm that the patch in #5 works.

Nice find! I noticed this once before but did not track down why it was happening. The fix seems correct.

I think we could provide test coverage to this to make sure future system blocks also get the correct category. The test might load the translated admin listing, assert that expected blocks are displayed, and assert that the untranslated word "system" does not appear.

Alternately (or additionally), is declaring a block without a category even valid? Should we warn developers of this somehow?

I think that declaring a block without a category is valid, at least that's what I understand from the default empty value in the annotation. I haven't found out why and how (and I would like know if someone can illuminate me), but when we don't declare a category, it uses the module's name.

What's happening here is that the 5 blocks from the system module don't declared a category (so it uses 'System' as a fall back), and that two blocks from other modules declared there category as 'System' and made it translatable. I think the module's name fall back is OK.

On a different note, the admin label too is optional, and it doesn't have a fall back, so when you don't declare one it stays blank. Shouldn't we use the block's ID in that case for the admin label?

Sounds like a backwards incompatible change to require categories. But fixing this problem by adding categories sounds fine. So maybe we should test that all core blocks provide a category as a best practice, but not as a requirement.

Here's a test to check that every core block have a category.

Hmm, it seems like my test does not catch all blocks without a category.
I guess it's because the namespaces array is only filled with enabled modules.
Is there a way to get ALL namespaces?

Here's a way to find all block plugins: loading every (non-test) modules to fill the namespaces array.

Just found a better way to get a list of all modules, thanks to @heddn!

I started out by testing the patch from #20 using the Korean translation.

Before:

After:

So I would say that the patch works as designed.

Then I followed the code a bit down the call stack and made a new patch. The only change is in core/lib/Drupal/Core/Plugin/CategorizingPluginManagerTrait.php:processDefinitionCategory() where I've passed the retrieved module name into t(). This would allow all categories that haven't been set to be translatable.

Changed the test to expect a TranslatableMarkup object instead of a string as default category.

The category was accidentally removed from PageTitleBlock.

Uploading re-rolled patch. It fixes the bug (see attached screenshot), but I haven't ran the tests to see if anything breaks. Lots of changes between 8.4.x and 8.7.x!

Ran tests -- the 2 that had been altered passed. All the reroll changes were the same as in the 8.4.x patch so marking this RTBC. Great job on this, looks great!

+++ b/core/lib/Drupal/Core/Plugin/CategorizingPluginManagerTrait.php
@@ -31,7 +31,7 @@ protected function processDefinitionCategory(&$definition) {
-      $definition['category'] = $this->getProviderName($definition['provider']);
+      $definition['category'] = $this->t($this->getProviderName($definition['provider'])); ¶

I'm not sure this is correct. The @translation should pass a TranslationMarkup object into the definition.

Also we should have the test that ensures all the blocks have categories that was in #27.

This is still relevant and the patch applies to 9.5.x.