Convert theme_html_tag() to Twig

patch for core.

#1: convert_theme_html_tag_to_twig-1939104-1.patch queued for re-testing.

apply for a new patch

edit Issue fixed via #1942490: Make Twig engine available during install

html_tag is used to create a head tag on install (maintenance) page.

Twig is registered via CoreBundle build(), registerTwig(); it is not called during install.php by the time that error is produced.

It works if add

use Symfony\Component\DependencyInjection\Definition;

and copy and paste contents of registerTwig() to install.core.inc a bit after line 334

    $container->register('twig.loader.filesystem', 'Twig_Loader_Filesystem')
      ->addArgument(DRUPAL_ROOT);
    $container->setAlias('twig.loader', 'twig.loader.filesystem');

    $container->register('twig', 'Drupal\Core\Template\TwigEnvironment')
      ->addArgument(new Reference('twig.loader'))
      ->addArgument(array(
        // This is saved / loaded via drupal_php_storage().
        // All files can be refreshed by clearing caches.
        // @todo ensure garbage collection of expired files.
        'cache' => settings()->get('twig_cache', TRUE),
        'base_template_class' => 'Drupal\Core\Template\TwigTemplate',
        // @todo Remove in followup issue
        // @see http://drupal.org/node/1712444.
        'autoescape' => FALSE,
        // @todo Remove in followup issue
        // @see http://drupal.org/node/1806538.
        'strict_variables' => FALSE,
        'debug' => settings()->get('twig_debug', FALSE),
        'auto_reload' => settings()->get('twig_auto_reload', NULL),
      ))
      ->addMethodCall('addExtension', array(new Definition('Drupal\Core\Template\TwigExtension')))
      // @todo Figure out what to do about debugging functions.
      // @see http://drupal.org/node/1804998
      ->addMethodCall('addExtension', array(new Definition('Twig_Extension_Debug')));

it works.. but that's a lot of copy and pasted code and seems quite unideal.

Saw you posted in #1825788: Symfony fatal when changing maintenance theme about this also, going to link to this comment there.

Just to see that tests will past, adding patch with the copy and pasted items

That's... odd; just manually went through site install and worked fine.

@hefox, thanks for explaining this issue, your patch worked on my local machine, but there was some issues like the css/js not loaded.

Last patch was missing the .html.twig *cross fingers*

This is failing (at least for first test) because the tests are looking for <link rel="canonical" but now it looks like <link class="" rel="canonical"

From what I recall, Jen etc. want that class=X in there to make it easier for newer people to understand where to add new classes. Personally, I dislike an attribute appearing when empty like that.

So, either class="" needs to not appear or the tests need to be updated/changed or a third+ possibility I haven't thought of.

This passes, locally. Here's hoping. I cheated a bit by letting the {{ attributes }} render the class as we are doing it now so there will be no empty class in the markup. Also, didn't include that registerTwig duplication above.

This whole thing may even just go away due to #1825090: Remove theme_html_tag() from the theme system

Also nice work @hefox and @artofeclipse for getting this started!

Oh also didn't include an interdiff because the patch in #12 didn't apply, I manually merged.

hmm, turns out was running tests on wrong site... this should be closer, but still may fail. And the whitespace modifiers were added strategically to pass the tests for ending newline.

Oh come on, give me a break, one fail "Fatal Error" on HtmlTagUnitTest.php Line 27

The whitespace modifiers I added purposefully leave the ending control's whitespace for a new line. I am going to re-run the tests but if it fails again, maybe someone else can have a look?

#18: twig-html-tag-1939104-18.patch queued for re-testing.

stealing this for review.

At first I couldn't figure out why there were tests for this theme function - I mean, why don't we have tests for the preprocess function of every other theme function? what makes this special?

- // Test auto-closure meta tag generation

I think that's the clue.

I believe the point of testThemeHtmlTag() is not to check that the tag coming into the theme function matches the tag coming out but to check that if you provide a "value" you end up with rendered markup that is "regular" HTML tag like <footag>value</footag> and if you provide no value you end up with a "self closing" HTML tag like <footag attribute="bar" />.

The new testPreprocessHtmlTag() intended to replace testThemeHtmlTag() is a much weaker test, it simply checks a few implementation details of the preprocess that have no real bearing on the final rendered HTML tag being closed correctly.

I'm going to say this needs work until the modifications to the tests are "comparing apples to apples".

Looks good to me :) Thanks for working on that test, it looks a lot more useful to me now.

+++ b/core/includes/theme.incundefined
@@ -2434,41 +2434,37 @@ function theme_feed_icon($variables) {
+  $variables['attributes'] = isset($element['#attributes']) ? new Attribute($element['#attributes']) : new Attribute(array());
+  $variables['tag'] = $element['#tag'];
+  if (isset($element['#value'])) {
+    $variables['value'] = $element['#value'];

This looks like another preprocess that could benefit from a switch from 'render element' to 'variables'.

+++ b/core/modules/system/lib/Drupal/system/Tests/Theme/HtmlTagTest.phpundefined
@@ -0,0 +1,36 @@
+class HtmlTagTest extends WebTestBase {

+++ /dev/nullundefined
@@ -1,36 +0,0 @@
-class HtmlTagUnitTest extends UnitTestBase {

Why the switch? This is :(