Breadcrumb for local tasks of Custom Blocks should contain the name of the custom block not the entity ID

Added _title_callback and pointed to the default EntityController title function.

Label function is called to generate breadcrumb title

The magic happens in TitleResolver->getTitle, it checks if route has _title_callback' or '_title' parameter.

Thank you for working on this!

Can confirm the change in the MR addresses the issue.

Could use a small test case to verify this too.

Probably can be added to BlockContentRevisionsTest

I wonder if most of block_content.routing.yml should be replaced with a Route provider annotation in the entity-types.

Route providers didn't exist at the time the entity/routes were added, but they do now.

We would need to retain the old route IDs which unfortunately don't follow the entity.{entity_type}.{link_template} naming convention.

#2919891: Make Vocabulary use a route provider instead of hard-coded routes provides an example conversion, the only difference for block content would be the route IDs.

Thoughts on a follow up to do that?

In terms of adding tests, \Drupal\Tests\block_content\Functional\PageEditTest::testPageEdit would be the best place

You will likely need to add and place the breadcrumb block (like we do for the title block in the setup method).

Added related issue that does the route provider work already

I have updated test script but next I had some trouble with:

core/modules/system/tests/src/Functional/Menu/AssertBreadcrumbTrait.p
hp (in context of class
Drupal\Tests\block_content\Functional\PageEditTest)
------ -----------------------------------------------------------------------
42 Variable $goto in isset() always exists and is not nullable.

First, I have fixed that but next got many PHPstan issues, added another ignore entry to phpstan-baseline.neon
I think the best would be to fix AssertBreadcrumbTrait.php, update phpstan-baseline.neon. Any recommendation how to deal witch such issue?

Removing the needs tests as I see you did that. Thanks!

Can you update assertBreadcrumb and remove the isset(), searching the repo I couldn't find an instance of nothing being passed in. Would also require removing (optional) from the doc. But lets see what breaks.

Reading the document block it saying $goto is optional but trail is not doesn't fully make sense. You would have to pass something as the first parameter to just pass in trail right?

But don't think testing can hurt.

isset() has been removed from AssertBreadcrumbTrait

Fact it passes makes me thing it was a true unneeded check.

Thanks for all the work you've done on this!

Looks good to me so sending to the next level.

Discussed with @alexpott and we agreed that the fix for the PHPStan warning wasn't right. The fix would have caused bunch of contrib testing to break: https://git.drupalcode.org/search?group_id=2&repository_ref=&scope=blobs.... If we want to fix the PHPStan warning, we should probably just change the docs to allow null. We have already marked it as optional, so in practice it is an allowed type even though it's not listed.

Either way, we should probably avoid making this change here, and instead file a new issue for this. I reverted the most recent commit that introduced these changes. I will wait until there is a passing test run.

Committed bfe3f5d and pushed to 10.1.x. Thanks!

Hey @lauriii, just opened a followup as per #17, it would be nice to have a review there.
#3336536: Fix PHPStan warning on assertBreadcrumb when passing NULL