Fix broken page title Update Manager (authorize.php)

Escaped HTML for links? will be taken care of in #1885564: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig

Note that #3 (links going to /core/authorize.php) is also partially fixed in the latest patches at #1885564: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig (the reason being that it touches the same code needed to fix the double escaping, #2).

If we leave it in that issue rather than this one, that leaves this:

1. "Authorize file system changes" as a title?

Which I'm not so convinced is a novice issue. We'd need to figure out why the session variable isn't being used in this code, which is therefore causing it to fall back to the default title:

  if (isset($_SESSION['authorize_page_title'])) {
    $page_title = $_SESSION['authorize_page_title'];
  }
  else {
    $page_title = t('Authorize file system changes');
  }

Was maybe semi-novice in the end :) Fixed by putting back some code that existed in Drupal 7 but was removed in #2192653: Remove drupal_set_title from authorize.php. Added a test also.

Thanks @stefan.r. This looks to work well, I tried install and update a module.

That will break the FTP/SSH case, though - since in that case authorize.php displays a form before the batch ever runs, and the form needs the correct title too.

Maybe we should do the opposite instead, though (always set the title at the beginning of the batch, never at the end)?

Oh actually we can't do that though (at least not easily), since system_authorized_init() is not used in those cases.

This goes back to #8, but with a code comment explaining why we can't remove the page titles from the system_authorized_init() calls. The interdiff is from #8.

My comment in #16 is wrong - we actually can do it easily (just set the title at the beginning of the batch process rather than the end). But it turns out there's already code trying to set a different title at the beginning of the batch process to use while the batch is running. It wasn't working though. So even with the above patches applied, you still see "Authorize file system changes" on the screen while the batch is in the middle of running.

The attached should make it work as desired all around. So the pattern now is:

1. At the beginning of the batch process, if a form is displayed (only true in the FTP/SSH case) the page title is "Update manager".
2. While the batch is running the title is either "Installing [project]" or "Installing updates".
3. When the batch is complete, the title is "Update manager".

Note that for manually testing this, you may need to log out and log back in again each time (otherwise the title from the previous round may still be in your session and get used as a result of that, thereby making the bug look like it's fixed when it actually isn't).

@David_Rothstein can you elaborate on this point you made in #18?

Note that for manually testing this, you may need to log out and log back in again each time (otherwise the title from the previous round may still be in your session and get used as a result of that, thereby making the bug look like it's fixed when it actually isn't

Will that introduce bugs for people installing multiple projects sequentially?

I should have clarified - I ran into that issue when testing the current 8.0.x behavior only (without the patch). Since some testing scenarios put the title in $_SESSION and others didn't, it was possible to run one scenario and think it worked correctly, but in actuality it only worked correctly because you had the title in $_SESSION from before.

With the patch, I believe the title gets set explicitly in every case, so if there's already something in $_SESSION it will be overwritten with the correct value. So I don't think you can run into that problem anymore.

Technically there is probably a small (existing) bug here that the page title in $_SESSION never gets unset once authorize.php is done using it... But after this patch I don't think that will matter in practice.

Thanks for the clarification because when I attempted to reproduce that, I couldn't:)

+1 to fix bug

+++ b/core/modules/update/update.authorize.inc
@@ -107,6 +110,11 @@ function update_authorize_run_install($filetransfer, $project, $updater_name, $l
+  $_SESSION['authorize_page_title'] = t('Installing %project', array('%project' => $project));

@@ -249,6 +257,7 @@ function update_authorize_update_batch_finished($success, $results) {
   $_SESSION['authorize_results']['messages'] = $results['log'];
   $_SESSION['authorize_results']['tasks'] = $results['tasks'];
+  $_SESSION['authorize_page_title'] = t('Update manager');

@@ -309,6 +318,7 @@ function update_authorize_install_batch_finished($success, $results) {
   $_SESSION['authorize_results']['messages'] = $results['log'];
   $_SESSION['authorize_results']['tasks'] = $results['tasks'];
+  $_SESSION['authorize_page_title'] = t('Update manager');

it needs follow-up to bring sanity.
"installing..." is a state of results so should live in that array key

Moving back to RTBC on the assumption that failure wasn't real.

Awesomesauce.

Committed and pushed to 8.0.x. Thanks!