PageTitle block is non-functional when not handled directly by \Drupal\block\Plugin\DisplayVariant\BlockPageVariant

Firstly, I can confirm this is an issue.

I see use cases where Layout Builder is used to take over the complete rendering of the page. It is most efficient if Front-enders can just style one block type and don't have to use two different approaches when using Layout Builder on some (entity) pages and regular regions on others.
So I'm in favour of fixing this instead removing it from the listing.

Also, when removing this, all the work in #2959962: Page Title block for Layout Builder does not have a place holder would be in vain.

Here's a proposed fix. Though @tedbow and I really are feeling a lot of deja vu around this issue...

Paired on this with @bnjmnm to write a test.
The FAIL patch is equivalent to an interdiff.

+++ b/core/lib/Drupal/Core/Block/Plugin/Block/PageTitleBlock.php
@@ -46,7 +109,7 @@ public function defaultConfiguration() {
+      '#title' => !is_null($this->title) ? $this->title : $this->titleResolver->getTitle($this->request, $this->routeMatch->getRouteObject()),

(I know you're updating the IS, but here's already a review of just the patch without having knowing the surrounding motivations.)

We tried very hard to only have the title resolver. But … there were use cases where this was deemed impossible. The canonical example was a page view whose title included the count of matching results. The only way to compute that is to execute the view; the title cannot be resolved independently. _{This is all from memory, I hope I'm remembering it correctly.} Looks like it's still there: \Drupal\views\Plugin\views\display\Page::execute().

If we can do this, then we should be able to simply do

'#title' => $this->titleResolver->getTitle($this->request, $this->routeMatch->getRouteObject()),

… which I think we'd all vastly prefer :) But until #2403359: Use _title and _title_callback where possible is fixed, which blocks #2359901: Discourage $main_content['#title'] in favor of route titles and title callbacks, I don't think we can do this. Unless we're willing to except that on certain pages, the page title may be incorrect if the title block is placed using Layout Builder…

Unless we're willing to except that on certain pages, the page title may be incorrect if the title block is placed using Layout Builder…

Precisely. I think with #2359901: Discourage $main_content['#title'] in favor of route titles and title callbacks in mind and the complexity of trying to respect #title for an unknown number of consumers, this is an acceptable improvement.

I added @todos pointing to that issue and rewrote the IS.

Thanks for the patch. I have gone through the code a couple times and not seeing anything obviously wrong except perhaps an extra space (below). I assume I just test this by adding a page title block so I'll try that.

+++ b/core/lib/Drupal/Core/Block/Plugin/Block/PageTitleBlock.php
@@ -46,7 +111,9 @@ public function defaultConfiguration() {
+      //   https://www.drupal.org/node/2359901 is resolved.

Nitpick: Remove extra space before https?

I tested adding a page title block and it is adding the block but not the label of the block. I assume there is already an issue for that but I'll move back to "Needs work" just in case.

While this is still very important, I have moved the stable blocker tag to a short-term fix: #3029819: Do not allow Page Title block to be placed in Layout Builder until it works properly

I signed off on this in Slack also, but explicitly documenting a +1 to #14 here. :)

However, promoting to major which I think is the correct priority here.

The current patch in #11 doesn't appear to be working against 8.7.

1) Patch has tests
2) Patch applies cleanly to 9.1.x branch
3) Patch applies cleanly to 9.0.x branch
what is left?

ah looks like there was some reason mentioned above for needs work.

good news is, patch still applies cleanly to 9.0.x
and 9.1.x and has tests.

I applied patch #14.

However the title field does not seem to respecting the language settings, the default title language is displayed instead. Whereas the other fields being displayed do match the language.

For example:
In English (default):
- Title field is in English
- Body field is in English

In Italian:
- Title field is in English <-- this should be in Italian.
- Body field is in Italian

I posted this: #3251104: Page Title block is not available in Layout Builder for User profile. earlier today reporting that the Page Title block was not available in Layout Builder for the User profile page. Someone closed that issue stating this one as a duplicate.

I applied this patch to a clean D9.2.7 site and the Page Title block is still not available when using Layout Builder to layout the User profile page.

Would this issue be expected to fix that?

This patch re-roll applies to Drupal 10.1 and 9.3 (and perhaps others too). It replaces core with core_version_requirement.

added default theme and Fixing CCF #29

It looks like the new files are missing from the patch in #30.

don't know why its not creating the patch file for where i actually made changes in file. according to CCF (Drupal\Tests\BrowserTestBase::$defaultTheme is required) i added default theme in (core/tests/Drupal/FunctionalTests/Core/Block/PageTitleBlockTest.php) file but after changes in this file when i run (git status) to check which file is actually changed it showing some different file that is (core/lib/Drupal/Core/Block/Plugin/Block/PageTitleBlock.php) and it is not showing the that exact file where i made changes.

Started at #29 as #30 and #32 removed the tests.

Also undid the changes from https://git.drupalcode.org/project/drupal/-/commit/ca8a550c056875254ff00...

Am getting this weird phpstan error not sure why

Running PHPStan on changed files.
 ------ --------------------------------------------------- 
  Line   modules/layout_builder/layout_builder.module       
 ------ --------------------------------------------------- 
  40     Function t invoked with 2 parameters, 1 required.  
  52     Function t invoked with 2 parameters, 1 required.  
  53     Function t invoked with 2 parameters, 1 required.  
  57     Function t invoked with 2 parameters, 1 required.  
  61     Function t invoked with 2 parameters, 1 required.  
 ------ --------------------------------------------------- 

Found 1 bug though, moving the block makes it disappear. Refreshing the page I see the block again and it moved so something weird is going on.

Fixing test failure and adding fix for moving block failure I mentioned in #35

But imagine tests will need to be updated based on the fix.

I think this should be postponed on #3370946: Page title should contextualize the local navigation

@tim.plunkett sounds good! If you get time mind reviewing though? Had to put a number of checks for layout builder, moving of blocks, and just regular titles.

Added some additional tests.

Re-roll from #29 against D10.2.x as the interdiffs it seemed I had but didn't have time to look further.

Ooops wrong patch.

Re-roll from #29 against D10.2.x.

Ok, there's a deprecation on str_contains when using PHP 8.1/PHP 8.2+, in some case came out null, so this is an interdiff and new patch making sure that it's an empty string that is used instead of a null.