Convert html.tpl.php to Twig

The exception message:
page_top could not be found in _context in "core/modules/system/templates/html.html.twig" at line 67

can be removed by using

{% if page_top %}
  {{ page_top }} 
{% endif %}

#12 - I get that too! but for every issue I work on and it's been happening for months.

#11: I have some questions..

why do we need to drupal_render the page top/bottom in a process function?

why do we need a process function?

also, please remove any references to PHP data types from Twig template docs.

#16 - I'm happy with those revisions to the docs and your explanation of the process stuff.

Is this really a manual testing issue?

If this template didn't work at all you'd be looking at about 50k failed tests. If there's some part of the template you're worried about, like a valid DOCTYPE or attributes being applied to the html and body tags correctly, and we don't already have automated tests for those then we should be writing them right now as they're very important to get right.

looking into tests.

There's an error being thrown on my local:

"User warning: page_top could not be found in _context in "core/modules/system/templates/html.html.twig" at line 49 in Drupal\Core\Template\TwigTemplate->getContextReference() (line 50 of core/lib/Drupal/Core/Template/TwigTemplate.php)."

It only seems to appear when navigating the admin sections using the Overlay - I think overlay_page_alter() or a related function is causing trouble. The error appears in either the overlay or in the parent page behind the overlay. Seeing the error and refreshing the page can cause it to disappear if the overlay is not active. If the overlay is active and the error has not appeared then refreshing the page causes it to appear in the parent page.

Adding this to the template resolves the issue for me but I don't know enough about the problem to know whether this is the best solution:

    {% if page_top %}
      {{ page_top }}
    {% endif %}
    {{ page }}
    {% if page_bottom %}
      {{ page_bottom }}
    {% endif %}

I guess I also answered my own question.

reassigning this to myself, OKed with @cottser.

As @thedavidmeister was suspecting, the overlay module is causing the error.

overlay_page_alter() is setting the "#access" flag to FALSE in the "page_top" renderable array, since it identifies it as a region to be skipped.

/**
 * Implements hook_page_alter().
 */
function overlay_page_alter(&$page) {
  // If we are limiting rendering to a subset of page regions, deny access to
  // all other regions so that they will not be processed.
  if ($regions_to_render = overlay_get_regions_to_render()) {
    $skipped_regions = array_diff(element_children($page), $regions_to_render);
    foreach ($skipped_regions as $skipped_region) {
      $page[$skipped_region]['#access'] = FALSE;
    }
  }
...

this is causing drupal_render() to return NULL further downstream in template_process_html() when it is trying to render "page_top".

Not sure what to do about this, other than checking for a truthy value of "page_top" before outputting it in the template.
("page_bottom" does not seem to be affected by that, so I left it as-is)

#24: 1961872-24.patch queued for re-testing.

#24: 1961872-24.patch queued for re-testing.

Wrapping the page_top in an if inside the Twig template is a hack, especially if it's undocumented and the page_bottom isn't also wrapped in an if - Apologies for suggesting it in the first place.

FWIW, I tried to replicate the same bug for page_bottom and could only do it when I removed $variables['page_bottom'] .= drupal_get_js('footer'); from the process function - showing that this really is just an issue with data types, and a clue for a fix :)

What about casting the variables to strings in template_process_html() instead, like this:

// Render page_top and page_bottom into top level variables.
$variables['page_top'] = isset($variables['page']['page_top']) ? (string) drupal_render($variables['page']['page_top']) : '';
$variables['page_bottom'] = isset($variables['page']['page_bottom']) ? (string) drupal_render($variables['page']['page_bottom']) : '';

Or even this - #1975442: drupal_render() should always return a string..

As outlined in comment #30, I removed the conditional from the template.

For the sake of moving along, I added type casting to template_process_html().

Modifying drupal_render() to always return a string is clearly the way to go. If and when this passes then the type casting becomes obsolete and should be removed.

#31 - This #1975462: Allow and test for NULL and integer 0 values in Twig templates. would help too.

#1975442: drupal_render() should always return a string. just got committed. We should be able to remove the type casting now.

removed type casting.

+ * variable, already prepared to be output as TITLE tag. Ths variable may

Ths?

+ * - slogan: The slogan of the site, if any, and if there is no title.

This sentence is wildly awkward.

 * - page: The rendered page content.
+ * - page_bottom: Final closing markup from any modules that have altered the
+ *   page. This variable should always be output last, after all other dynamic
+ *   content.
+ * - page_top: Initial markup from any modules that have altered the page. This
+ *   variable should always be output first, before all other dynamic content.

I feel like these variables could be better documented in the order they are required to be rendered - page_top, page, page_bottom.

Made documentation changes as suggested in #35.

Interdiff with respect to #34.

I can only see two little nitpicks:

  * @see system_elements()
- * @see html.tpl.php
+ * @see html.html.twig

There's no need to @see the twig file that this function belongs to. That's what the 'Default template' statement is for.

  * @see template_preprocess_html()
- * @see html.tpl.php
+ * @see html.html.twig

It's enough to just refer to the preprocess, not the template.

Fixed the two minor points from #39

And from #40

RTBC then?

+1

We'll cleanup scattered docs later.

remove sand

The conversion looks good... we just need to some profiling done.

+1. Ready for #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch

Fixing title...

Committed 8ec70c2 and pushed to 8.x. Thanks!