Batch missing title on screen

This seems to have been the subject of another issue before. #2282035: Title not shown during a batch operation

I'll give this a try ;)

@hctom,

Just un-assigning the issue. But, you can continue working on the issue though. :)

Thanks!!

Okay, I digged into this issue, and found out, that the header region is not set in the BatchController class. So when this is empty, no header content will be rendered. See HtmlRenderer class (lines 262 and following):

    // $page is now fully built. Find all non-empty page regions, and add a
    // theme wrapper function that allows them to be consistently themed.
    $regions = \Drupal::theme()->getActiveTheme()->getRegions();
    foreach ($regions as $region) {
      if (!empty($page[$region])) {
        $page[$region]['#theme_wrappers'][] = 'region';
        $page[$region]['#region'] = $region;
      }
    }

As a fix, I now injected a page_title render element into the header region (if a title is present), which is then rendered at the top of the page. I am not really sure, if this is more like a hacky solution, or if this is the way to go.

So please review the patch attached and feel free to provide feedback/customizations.

@th_tushar: Why did you un-assign the issue? Fortunately nobody else was also trying to fix this the same time...

I have reviewed the patch and it seems good. I would suggest the attached change to it as a code-quality improvement however, which simplifies title processing and makes it clear that the same value is being used for page title and header.

I'm at DrupalCon Vienna, first time sprinter, giving this one a go :)

Actioned as per John's comments #11

Re-done patch due diff against wrong branch.

Thanks everyone for the work so far here! The screenshots also help explain the issue.

I was just looking into the possibility of adding automated test coverage for this bug. It might be tricky to test during the batch test run, but we do have tests for the start and the end in core/modules/system/tests/src/Functional/Batch/ProcessingTest.php. Is the page title gone the whole time? Can we add test assertions that fail in HEAD, but pass when combined with our fix here?

It might also be that the title is only missing during the middle of the batch running, but let's check.

Updated the page title test to check the text exists in the HTML.

Applied patch from #20 and it worked as expected. The titile is appearing for the batch properly. Attached the before and after screenshot.

Awesome to see this getting fixed. One of those weird little things you're not even sure is an issue but you keep wondering the same thing every time you see it. Awesome work everyone!

Must be mentioned test on #20 is altering an existing testBatchProgressPageTitle() test, which doc block reads: "Tests that the batch API progress page shows the title correctly."

That bit was introduced on #2282035: Title not shown during a batch operation, but to me it doesn't do what it says. It tests the title can be resolved, but not that the title shows correctly (in fact we have the bug this issue is trying to fix :) ).
Do we understand the reasons why that has been done? Is it still relevant?
If relevant, should we tidy up and maybe split the tests in 2, one checking the title can be resolved and one checking the title shows up correctly?
If not, should we remove the not-relevant bit and keeping only last test?

#20 looks like testing only the initial iteration ($this->maximumMetaRefreshCount = 0). I'm not 100% confident with Batch API but is it worth checking title is still there while batch is in progress, i.e. $this->maximumMetaRefreshCount = 1?

+++ b/core/modules/system/tests/src/Functional/Batch/PageTest.php
@@ -50,7 +50,15 @@ public function testBatchProgressPageTitle() {
+    $this->assertText('Batch Test', 'The test is in the html output.');

assertText is deprecated however there are already uses of it in this function so to minimize scope creep, not going to ask for it to be removed.

Committed as 3442528 and pushed to 8.5.x

Cherry-picked as 8db10b6 and pushed to 8.4.x.

Thanks all