Move Title from process to preprocess, remove template_process_page()

Attached patch is moving title, breadcrumb, messages to preprocess layer from template_process_page.

I'm not familiar with what has been discussed regarding template variable processing for Twig templates, but since Twig is only one possible template engine, I don't think that this is the right approach.

The reason for why we have the separate process step is to properly account for race conditions.

I.e., you are no longer able to adjust/inject/alter the breadcrumb, if it is rendered/processed too early. You are no longer able to change the page title through regular functions, if it is rendered too early. You are no longer able to output any status/info/warning/error messages, if they are rendered too early (they will appear on the next page request instead).

+++ b/core/includes/theme.inc
@@ -2634,22 +2637,7 @@ function template_preprocess_page(&$variables) {
-  if (!isset($variables['breadcrumb'])) {
-    // Build the breadcrumb last, so as to increase the chance of being able to
-    // re-use the cache of an already rendered menu containing the active link
-    // for the current page.
-    // @see menu_tree_page_data()
-    $variables['breadcrumb'] = theme('breadcrumb', array('breadcrumb' => drupal_get_breadcrumb()));
-  }

Also, this potentially has a negative impact on performance.

+++ b/core/includes/theme.inc
@@ -2634,22 +2637,7 @@ function template_preprocess_page(&$variables) {
-  if (!isset($variables['title'])) {
-    $variables['title'] = drupal_get_title();
-  }

This is directly related to the breadcrumb processing, since the page title is derived from the active trail (~breadcrumb) by default, unless a manual page title has been set via drupal_set_title() in the page building process.

+++ b/core/includes/theme.inc
@@ -2634,22 +2637,7 @@ function template_preprocess_page(&$variables) {
-  // Generate messages last in order to capture as many as possible for the
-  // current page.
-  if (!isset($variables['messages'])) {
-    $variables['messages'] = $variables['show_messages'] ? theme('status_messages') : '';
-  }

The inline comment already clarifies why this is done as the very very last step of template variable processing:

If you move this earlier, debug(), dsm(), dpm(), drupal_set_message(), and many other functions cease to work when being called from template preprocess functions.

Agreed. The stuff in template_process_page is there only because it is a set of things that are affected by the rendering of the rest of the page. Adding css files during preprocess, catching as many messages as possible, messing with the page title during preprocess, messing with breadcrumb during preprocess.

For page-level it seems like it's still necessary to have some special cases for these things, or maybe do a second pass for these...

Agreed with the RenderWrapper idea - here is a patch that combines #2004286-60: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class with using the RenderWrapper for drupal_get_title(). Second patch is without the code from the other issue.

Yes, the -do-not-test patch depends on #2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class. I think we could remove template_process_page() on the (to be revised) patch on the meta.

Edit: Although I guess that would leave us with an empty template_process_page()…

Needs a test for the new RenderWrapper

Ok, tests added for RenderWrapper earlier today: #2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class. 1589968-8-do-not-test.patch depends on that issue and still applies fine.

So back to RTBC per #9 but #2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class would need to be committed first.

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

#2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class has been committed so lets get a patch based on the do not test patch in #8 tested...

1589968-8-do-not-test.patch in #8 still applied.
Just a rename, so it gets tested.

Applied the patch to
HEAD is now at b416a37 Issue #1941286 by scor, Eric_A: Remove the process layer (rdf module).

Title, Slogan and breadcrumbs are working. Cheers

And back to RTBC

#2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class has been reverted...

Ho hum... no it's not... I'm just going to postpone this on #2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class Cottser's work on adding tests to that issue is important.

#15: 1589968-8.patch queued for re-testing.

Needs a quick reroll.

rerolled

removed needs reroll tag

Title is all that's left in template_process_page() now that #1843668: Move building of breadcrumb render array from template_process_page() to template_preprocess_page() has been committed, so this patch should actually remove the whole template_process_page() function now.

And technically it needs a reroll as well :)

Rerolled. Did a quick S&R for template_process_page and removed/changed references to it in any remaining docs.

.

This is RTBC - provided testbot is green!

Test passed

Committed 5d649f8 and pushed to 8.x. Thanks!