API Move createTag to HTML class

ENHANCEMENT Better HTML generation behaviour for Requirements_Backend

_config/html.yml

@@ -2,7 +2,9 @@
 Name: corehtml
 ---
 SilverStripe\Core\Injector\Injector:
-  HTMLValue:
+  SilverStripe\View\Parsers\HTMLValue:
     class: SilverStripe\View\Parsers\HTML4Value
+  # Shorthand
+  HTMLValue: '%$SilverStripe\View\Parsers\HTMLValue'
   SilverStripe\Forms\HTMLEditor\HTMLEditorConfig:
     class: SilverStripe\Forms\HTMLEditor\TinyMCEConfig

docs/en/04_Changelogs/4.0.0.md

@@ -1695,6 +1695,9 @@ The following filesystem synchronisation methods and tasks are also removed
 * Added method `FormField::setSubmittedValue($value, $data)` to process input submitted from form
   submission, in contrast to `FormField::setValue($value, $data)` which is intended to load its
   value from the ORM. The second argument to setValue() has been added.
+* `HTMLValue` service name is now fully qualified `SilverStripe\View\Parsers\HTMLValue`.
+* FormField::create_tag() has been moved to `HTML` class and renamed to createTag. Invoke with
+  `HTML::createTag()`.
 
 The following methods and properties on `Requirements_Backend` have been renamed:
 

src/Forms/FormField.php

@@ -315,61 +315,6 @@ class FormField extends RequestHandler
         return preg_replace('/([a-z]+)([A-Z])/', '$1 $2', $label);
     }
 
-    /**
-     * Construct and return HTML tag.
-     *
-     * @param string $tag
-     * @param array $attributes
-     * @param null|string $content
-     *
-     * @return string
-     */
-    public static function create_tag($tag, $attributes, $content = null)
-    {
-        $preparedAttributes = '';
-
-        foreach ($attributes as $attributeKey => $attributeValue) {
-            if (!empty($attributeValue) || $attributeValue === '0' || ($attributeKey == 'value' && $attributeValue !== null)) {
-                $preparedAttributes .= sprintf(
-                    ' %s="%s"',
-                    $attributeKey,
-                    Convert::raw2att($attributeValue)
-                );
-            }
-        }
-
-        if ($content || !in_array($tag, [
-                'area',
-                'base',
-                'br',
-                'col',
-                'embed',
-                'hr',
-                'img',
-                'input',
-                'link',
-                'meta',
-                'param',
-                'source',
-                'track',
-                'wbr',
-        ])) {
-            return sprintf(
-                '<%s%s>%s</%s>',
-                $tag,
-                $preparedAttributes,
-                $content,
-                $tag
-            );
-        }
-
-        return sprintf(
-            '<%s%s />',
-            $tag,
-            $preparedAttributes
-        );
-    }
-
     /**
      * Creates a new field.
      *

src/Forms/GridField/GridField.php

@@ -19,6 +19,7 @@ use SilverStripe\Forms\FormField;
 use SilverStripe\Forms\Form;
 use LogicException;
 use InvalidArgumentException;
+use SilverStripe\View\HTML;
 
 /**
  * Displays a {@link SS_List} in a grid format.
@@ -466,9 +467,8 @@ class GridField extends FormField
         }
 
         // Display a message when the grid field is empty.
-
         if (empty($content['body'])) {
-            $cell = FormField::create_tag(
+            $cell = HTML::createTag(
                 'td',
                 array(
                     'colspan' => count($columns),
@@ -476,7 +476,7 @@ class GridField extends FormField
                 _t('SilverStripe\\Forms\\GridField\\GridField.NoItemsFound', 'No items found')
             );
 
-            $row = FormField::create_tag(
+            $row = HTML::createTag(
                 'tr',
                 array(
                     'class' => 'ss-gridfield-item ss-gridfield-no-items',
@@ -518,20 +518,20 @@ class GridField extends FormField
         );
 
         if ($this->getDescription()) {
-            $content['after'] .= FormField::create_tag(
+            $content['after'] .= HTML::createTag(
                 'span',
                 array('class' => 'description'),
                 $this->getDescription()
             );
         }
 
-        $table = FormField::create_tag(
+        $table = HTML::createTag(
             'table',
             $tableAttributes,
             $header . "\n" . $footer . "\n" . $body
         );
 
-        return FormField::create_tag(
+        return HTML::createTag(
             'fieldset',
             $fieldsetAttributes,
             $content['before'] . $table . $content['after']
@@ -549,7 +549,7 @@ class GridField extends FormField
      */
     protected function newCell($total, $index, $record, $attributes, $content)
     {
-        return FormField::create_tag(
+        return HTML::createTag(
             'td',
             $attributes,
             $content
@@ -567,7 +567,7 @@ class GridField extends FormField
      */
     protected function newRow($total, $index, $record, $attributes, $content)
     {
-        return FormField::create_tag(
+        return HTML::createTag(
             'tr',
             $attributes,
             $content
@@ -973,9 +973,7 @@ class GridField extends FormField
      *
      * @param HTTPRequest $request
      * @param DataModel $model
-     *
+     * @return array|RequestHandler|HTTPResponse|string
-     * @return array|RequestHandler|HTTPResponse|string|void
-     *
      * @throws HTTPResponse_Exception
      */
     public function handleRequest(HTTPRequest $request, DataModel $model)
@@ -1091,7 +1089,7 @@ class GridField extends FormField
     protected function getOptionalTableHeader(array $content)
     {
         if ($content['header']) {
-            return FormField::create_tag(
+            return HTML::createTag(
                 'thead',
                 array(),
                 $content['header']
@@ -1109,7 +1107,7 @@ class GridField extends FormField
     protected function getOptionalTableBody(array $content)
     {
         if ($content['body']) {
-            return FormField::create_tag(
+            return HTML::createTag(
                 'tbody',
                 array('class' => 'ss-gridfield-items'),
                 $content['body']
@@ -1127,7 +1125,7 @@ class GridField extends FormField
     protected function getOptionalTableFooter($content)
     {
         if ($content['footer']) {
-            return FormField::create_tag(
+            return HTML::createTag(
                 'tfoot',
                 array(),
                 $content['footer']

src/Forms/GridField/GridFieldLevelup.php

@@ -3,11 +3,11 @@
 namespace SilverStripe\Forms\GridField;
 
 use SilverStripe\Core\Injector\Injectable;
-use SilverStripe\Forms\FormField;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\ORM\Hierarchy\Hierarchy;
 use SilverStripe\View\ArrayData;
+use SilverStripe\View\HTML;
 use SilverStripe\View\SSViewer;
 
 /**
@@ -79,7 +79,7 @@ class GridFieldLevelup implements GridField_HTMLProvider
             'href' => sprintf($this->linkSpec, $parentID),
             'class' => 'cms-panel-link ss-ui-button font-icon-level-up no-text grid-levelup'
         ));
-        $linkTag = FormField::create_tag('a', $attrs);
+        $linkTag = HTML::createTag('a', $attrs);
 
         $forTemplate = new ArrayData(array(
             'UpLink' => DBField::create_field('HTMLFragment', $linkTag)

src/Forms/HTMLEditor/EmbedShortcodeProvider.php

@@ -4,7 +4,7 @@ namespace SilverStripe\Forms\HtmlEditor;
 
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
-use SilverStripe\Forms\FormField;
+use SilverStripe\View\HTML;
 use SilverStripe\View\Parsers\ShortcodeHandler;
 use Embed\Adapters\Adapter;
 use Embed\Embed;
@@ -132,7 +132,7 @@ class EmbedShortcodeProvider implements ShortcodeHandler
         unset($arguments['height']);
         unset($arguments['url']);
         unset($arguments['caption']);
-        return FormField::create_tag('div', $arguments, $content);
+        return HTML::createTag('div', $arguments, $content);
     }
 
     /**
@@ -151,7 +151,7 @@ class EmbedShortcodeProvider implements ShortcodeHandler
         unset($arguments['height']);
         unset($arguments['url']);
         $arguments['href'] = $href;
-        return Formfield::create_tag('a', $arguments, Convert::raw2xml($title));
+        return HTML::createTag('a', $arguments, Convert::raw2xml($title));
     }
 
     /**
@@ -165,6 +165,6 @@ class EmbedShortcodeProvider implements ShortcodeHandler
     {
         $arguments['src'] = $src;
         unset($arguments['url']);
-        return FormField::create_tag('img', $arguments);
+        return HTML::createTag('img', $arguments);
     }
 }

src/Forms/HTMLEditor/HTMLEditorField.php

@@ -3,11 +3,11 @@
 namespace SilverStripe\Forms\HTMLEditor;
 
 use SilverStripe\Assets\Image;
-use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Forms\TextareaField;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataObjectInterface;
 use Exception;
+use SilverStripe\View\Parsers\HTMLValue;
 
 /**
  * A TinyMCE-powered WYSIWYG HTML editor field with image and link insertion and tracking capabilities. Editor fields
@@ -129,7 +129,7 @@ class HTMLEditorField extends TextareaField
         }
 
         // Sanitise if requested
-        $htmlValue = Injector::inst()->create('HTMLValue', $this->Value());
+        $htmlValue = HTMLValue::create($this->Value());
         if (HTMLEditorField::config()->sanitise_server_side) {
             $santiser = HTMLEditorSanitiser::create(HTMLEditorConfig::get_active());
             $santiser->sanitise($htmlValue);

src/Forms/SelectionGroup.php

@@ -4,7 +4,7 @@ namespace SilverStripe\Forms;
 
 use SilverStripe\ORM\ArrayList;
 use SilverStripe\ORM\FieldType\DBField;
-use SilverStripe\View\Requirements;
+use SilverStripe\View\HTML;
 
 /**
  * Represents a number of fields which are selectable by a radio
@@ -92,7 +92,7 @@ class SelectionGroup extends CompositeField
             $itemID = $this->ID() . '_' . (++$count);
             // @todo Move into SelectionGroup_Item.ss template at some point.
             $extra = array(
-                "RadioButton" => DBField::create_field('HTMLFragment', FormField::create_tag(
+                "RadioButton" => DBField::create_field('HTMLFragment', HTML::createTag(
                     'input',
                     array(
                         'class' => 'selector',

src/ORM/FieldType/DBHTMLText.php

@@ -7,6 +7,7 @@ use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Control\HTTP;
 use SilverStripe\Forms\TextField;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorField;
+use SilverStripe\View\Parsers\HTMLValue;
 use SilverStripe\View\Parsers\ShortcodeParser;
 
 /**
@@ -177,7 +178,7 @@ class DBHTMLText extends DBText
     public function whitelistContent($value)
     {
         if ($this->whitelist) {
-            $dom = Injector::inst()->create('HTMLValue', $value);
+            $dom = HTMLValue::create($value);
 
             $query = array();
             $textFilter = ' | //body/text()';

src/View/HTML.php

@@ -0,0 +1,78 @@
+<?php
+
+namespace SilverStripe\View;
+
+use InvalidArgumentException;
+use SilverStripe\Core\Config\Configurable;
+use SilverStripe\Core\Convert;
+
+/**
+ * HTML Helper class
+ */
+class HTML
+{
+    use Configurable;
+
+    /**
+     * List of HTML5 void elements
+     *
+     * @see https://www.w3.org/TR/html51/syntax.html#void-elements
+     * @config
+     * @var array
+     */
+    private static $void_elements = [
+        'area',
+        'base',
+        'br',
+        'col',
+        'embed',
+        'hr',
+        'img',
+        'input',
+        'keygen',
+        'link',
+        'menuitem',
+        'meta',
+        'param',
+        'source',
+        'track',
+        'wbr'
+    ];
+
+    /**
+     * Construct and return HTML tag.
+     *
+     * @param string $tag
+     * @param array $attributes
+     * @param string $content Content to use between two tags. Not valid for void elements (e.g. link)
+     * @return string
+     */
+    public static function createTag($tag, $attributes, $content = null)
+    {
+        $tag = strtolower($tag);
+
+        // Build list of arguments
+        $preparedAttributes = '';
+        foreach ($attributes as $attributeKey => $attributeValue) {
+            // Only set non-empty strings (ensures strlen(0) > 0)
+            if (strlen($attributeValue) > 0) {
+                $preparedAttributes .= sprintf(
+                    ' %s="%s"',
+                    $attributeKey,
+                    Convert::raw2att($attributeValue)
+                );
+            }
+        }
+
+        // Check void element type
+        if (in_array($tag, static::config()->get('void_elements'))) {
+            if ($content) {
+                throw new InvalidArgumentException("Void element \"{$tag}\" cannot have content");
+            }
+            return "<{$tag}{$preparedAttributes} />";
+        }
+
+        // Closed tag type
+        return "<{$tag}{$preparedAttributes}>{$content}</{$tag}>";
+    }
+}

src/View/Parsers/Diff.php

@@ -42,7 +42,7 @@ class Diff extends \Diff
             $content = $cleaner->cleanHTML($content);
         } else {
             // At most basic level of cleaning, use DOMDocument to save valid XML.
-            $doc = Injector::inst()->create('HTMLValue', $content);
+            $doc = HTMLValue::create($content);
             $content = $doc->getContent();
         }
 

src/View/Parsers/PurifierHTMLCleaner.php

@@ -2,7 +2,6 @@
 
 namespace SilverStripe\View\Parsers;
 
-use SilverStripe\Core\Injector\Injector;
 use HTMLPurifier;
 
 /**
@@ -14,7 +13,7 @@ class PurifierHTMLCleaner extends HTMLCleaner
     public function cleanHTML($content)
     {
         $html = new HTMLPurifier();
-        $doc = Injector::inst()->create('HTMLValue', $html->purify($content));
+        $doc = HTMLValue::create($html->purify($content));
         return $doc->getContent();
     }
 }

src/View/Parsers/ShortcodeParser.php

@@ -624,7 +624,7 @@ class ShortcodeParser
 
         if ($content) {
             /** @var HTMLValue $parsed */
-            $parsed = Injector::inst()->create('HTMLValue', $content);
+            $parsed = HTMLValue::create($content);
             $body = $parsed->getBody();
             if ($body) {
                 $this->insertListAfter($body->childNodes, $node);
@@ -669,7 +669,7 @@ class ShortcodeParser
             // Now parse the result into a DOM
             if (!$htmlvalue->isValid()) {
                 if (self::$error_behavior == self::ERROR) {
-                    user_error('Couldn\'t decode HTML when processing short codes', E_USER_ERRROR);
+                    user_error('Couldn\'t decode HTML when processing short codes', E_USER_ERROR);
                 } else {
                     $continue = false;
                 }

src/View/Requirements_Backend.php

@@ -778,38 +778,55 @@ class Requirements_Backend
         // Combine files - updates $this->javascript and $this->css
         $this->processCombinedFiles();
 
+        // Script tags for js links
         foreach ($this->getJavascript() as $file => $attributes) {
-            $async = (isset($attributes['async']) && $attributes['async'] == true) ? " async" : "";
+            // Build html attributes
-            $defer = (isset($attributes['defer']) && $attributes['defer'] == true) ? " defer" : "";
+            $htmlAttributes = [
-            $type = Convert::raw2att(isset($attributes['type']) ? $attributes['type'] : "application/javascript");
+                'type' => isset($attributes['type']) ? $attributes['type'] : "application/javascript",
-            $path = Convert::raw2att($this->pathForFile($file));
+                'src' => $this->pathForFile($file),
-            if ($path) {
+            ];
-                $jsRequirements .= "<script type=\"{$type}\" src=\"{$path}\"{$async}{$defer}></script>";
+            if (!empty($attributes['async'])) {
+                $htmlAttributes['async'] = 'async';
             }
+            if (!empty($attributes['defer'])) {
+                $htmlAttributes['defer'] = 'defer';
+            }
+            $jsRequirements .= HTML::createTag('script', $htmlAttributes);
+            $jsRequirements .= "\n";
         }
 
         // Add all inline JavaScript *after* including external files they might rely on
         foreach ($this->getCustomScripts() as $script) {
-            $jsRequirements .= "<script type=\"application/javascript\">//<![CDATA[\n";
+            $jsRequirements .= HTML::createTag(
-            $jsRequirements .= "$script\n";
+                'script',
-            $jsRequirements .= "//]]></script>";
+                [ 'type' => 'application/javascript' ],
+                "//<![CDATA[\n{$script}\n//]]>"
+            );
+            $jsRequirements .= "\n";
         }
 
+        // CSS file links
         foreach ($this->getCSS() as $file => $params) {
-            $path = Convert::raw2att($this->pathForFile($file));
+            $htmlAttributes = [
-            if ($path) {
+                'rel' => 'stylesheet',
-                $media = (isset($params['media']) && !empty($params['media']))
+                'type' => 'text/css',
-                    ? " media=\"{$params['media']}\"" : "";
+                'href' => $this->pathForFile($file),
-                $requirements .= "<link rel=\"stylesheet\" type=\"text/css\" {$media} href=\"$path\" />\n";
+            ];
+            if (!empty($params['media'])) {
+                $htmlAttributes['media'] = $params['media'];
             }
+            $requirements .= HTML::createTag('link', $htmlAttributes);
+            $requirements .= "\n";
         }
 
+        // Literal custom CSS content
         foreach ($this->getCustomCSS() as $css) {
-            $requirements .= "<style type=\"text/css\">\n$css\n</style>\n";
+            $requirements .= HTML::createTag('style', ['type' => 'text/css'], "\n{$css}\n");
+            $requirements .= "\n";
         }
 
         foreach ($this->getCustomHeadTags() as $customHeadTag) {
-            $requirements .= "$customHeadTag\n";
+            $requirements .= "{$customHeadTag}\n";
         }
 
         // Inject CSS  into body
@@ -971,8 +988,8 @@ class Requirements_Backend
         $candidates = array(
             'en.js',
             'en_US.js',
-            i18n::getData()->langFromLocale(i18n::config()->default_locale) . '.js',
+            i18n::getData()->langFromLocale(i18n::config()->get('default_locale')) . '.js',
-            i18n::config()->default_locale . '.js',
+            i18n::config()->get('default_locale') . '.js',
             i18n::getData()->langFromLocale(i18n::get_locale()) . '.js',
             i18n::get_locale() . '.js',
         );

tests/behat/features/login.feature

@@ -1,4 +1,4 @@
-# features/login.feature
+@retry
 Feature: Log in
   As an site owner
   I want to access to the CMS to be secure

tests/behat/features/lostpassword.feature

@@ -1,4 +1,4 @@
-@todo
+@retry
 Feature: Lost Password
   As a site owner
   I want to be able to reset my password

tests/behat/features/manage-users.feature

@@ -1,4 +1,4 @@
-@javascript
+@javascript @retry
 Feature: Manage users
   As a site administrator
   I want to create and manage user accounts on my site

tests/behat/features/profile.feature

@@ -1,3 +1,4 @@
+@retry
 Feature: Manage my own settings
 	As a CMS user
 	I want to be able to change personal settings

tests/behat/features/security-permissions.feature

@@ -1,4 +1,4 @@
-@javascript
+@javascript @retry
 Feature: Manage Security Permissions for Groups
   As a site administrator
   I want to control my user's security permissions in an intuitive way

tests/php/Forms/FormFieldTest.php

@@ -5,7 +5,6 @@ namespace SilverStripe\Forms\Tests;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\ClassInfo;
 use SilverStripe\Dev\SapphireTest;
-use SilverStripe\Control\Controller;
 use SilverStripe\Forms\FormField;
 use SilverStripe\Forms\Tests\FormFieldTest\TestExtension;
 use SilverStripe\Forms\TextField;
@@ -13,7 +12,6 @@ use SilverStripe\Forms\RequiredFields;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\Form;
 use ReflectionClass;
-use SilverStripe\ORM\FieldType\DBField;
 
 class FormFieldTest extends SapphireTest
 {
@@ -371,14 +369,4 @@ class FormFieldTest extends SapphireTest
             $schema['message']['value']
         );
     }
-
-    public function testCreateVoidTag()
-    {
-        $tag = FormField::create_tag('meta', [
-            'name' => 'description',
-            'content' => 'test tag',
-        ]);
-        $this->assertNotContains('</meta>', $tag);
-        $this->assertRegexp('#/>$#', $tag);
-    }
 }

tests/php/Forms/HTMLEditor/HTMLEditorSanitiserTest.php

@@ -6,6 +6,7 @@ use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\FunctionalTest;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
 use SilverStripe\Forms\HTMLEditor\HTMLEditorSanitiser;
+use SilverStripe\View\Parsers\HTMLValue;
 
 class HTMLEditorSanitiserTest extends FunctionalTest
 {
@@ -53,7 +54,7 @@ class HTMLEditorSanitiserTest extends FunctionalTest
             $config->setOptions(array('valid_elements' => $validElements));
             $sanitiser = new HtmlEditorSanitiser($config);
 
-            $htmlValue = Injector::inst()->create('HTMLValue', $input);
+            $htmlValue = HTMLValue::create($input);
             $sanitiser->sanitise($htmlValue);
 
             $this->assertEquals($output, $htmlValue->getContent(), $desc);

tests/php/View/HTMLTest.php

@@ -0,0 +1,58 @@
+<?php
+
+namespace SilverStripe\View\Tests;
+
+use InvalidArgumentException;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\View\HTML;
+
+class HTMLTest extends SapphireTest
+{
+    public function testCreateVoidTag()
+    {
+        $tag = HTML::createTag('meta', [
+            'name' => 'description',
+            'content' => 'test tag',
+        ]);
+        $this->assertEquals('<meta name="description" content="test tag" />', $tag);
+    }
+
+    public function testEmptyAttributes()
+    {
+        $tag = HTML::createTag('meta', [
+            'value' => 0,
+            'content' => '',
+            'max' => 3,
+            'details' => null,
+            'disabled' => false,
+            'readonly' => true,
+        ]);
+        $this->assertEquals('<meta value="0" max="3" readonly="1" />', $tag);
+    }
+
+    public function testNormalTag()
+    {
+        $tag = HTML::createTag('a', [
+            'title' => 'Some link',
+            'nullattr' => null,
+        ]);
+        $this->assertEquals('<a title="Some link"></a>', $tag);
+
+        $tag = HTML::createTag('a', [
+            'title' => 'HTML & Text',
+            'nullattr' => null,
+        ], 'Some <strong>content!</strong>');
+        $this->assertEquals('<a title="HTML &amp; Text">Some <strong>content!</strong></a>', $tag);
+    }
+
+    public function testVoidContentError()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage("Void element \"link\" cannot have content");
+
+        HTML::createTag('link', [
+            'title' => 'HTML & Text',
+            'nullattr' => null,
+        ], 'Some <strong>content!</strong>');
+    }
+}

tests/php/View/RequirementsTest.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\View\Tests;
 
+use InvalidArgumentException;
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\Dev\SapphireTest;
@@ -34,9 +35,7 @@ class RequirementsTest extends SapphireTest
 
     public function testExternalUrls()
     {
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $backend->setCombinedFilesEnabled(true);
 
@@ -166,9 +165,7 @@ class RequirementsTest extends SapphireTest
 
     public function testCustomType()
     {
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $basePath = $this->getThemeRoot();
         $this->setupRequirements($backend);
@@ -194,9 +191,7 @@ class RequirementsTest extends SapphireTest
 
     public function testCombinedJavascript()
     {
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupCombinedRequirements($backend);
 
@@ -249,9 +244,7 @@ class RequirementsTest extends SapphireTest
 
         // Then do it again, this time not requiring the files beforehand
         unlink($combinedFilePath);
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupCombinedNonrequiredRequirements($backend);
         $html = $backend->includeInHTML(self::$html_template);
@@ -294,9 +287,7 @@ class RequirementsTest extends SapphireTest
 
     public function testCombinedJavascriptAsyncDefer()
     {
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
 
         $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, true, false);
@@ -371,9 +362,7 @@ class RequirementsTest extends SapphireTest
 
         // setup again for testing defer
         unlink($combinedFilePath);
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
 
         $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, false, true);
@@ -445,9 +434,7 @@ class RequirementsTest extends SapphireTest
 
         // setup again for testing async and defer
         unlink($combinedFilePath);
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
 
         $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, true, true);
@@ -456,7 +443,7 @@ class RequirementsTest extends SapphireTest
 
         /* ASYNC/DEFER IS INCLUDED IN SCRIPT TAG */
         $this->assertRegExp(
-            '/src=".*' . preg_quote($combinedFileName, '/') . '" async defer/',
+            '/src=".*' . preg_quote($combinedFileName, '/') . '" async="async" defer="defer"/',
             $html,
             'async and defer are included in script tag'
         );
@@ -520,9 +507,7 @@ class RequirementsTest extends SapphireTest
     public function testCombinedCss()
     {
         $basePath = $this->getThemeRoot();
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
 
@@ -551,9 +536,7 @@ class RequirementsTest extends SapphireTest
         );
 
         // Test that combining a file multiple times doesn't trigger an error
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
         $backend->combineFiles(
@@ -582,9 +565,7 @@ class RequirementsTest extends SapphireTest
     public function testBlockedCombinedJavascript()
     {
         $basePath = $this->getThemeRoot();
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupCombinedRequirements($backend);
         $combinedFileName = '/_combinedfiles/RequirementsTest_bc-2a55d56.js';
@@ -622,14 +603,12 @@ class RequirementsTest extends SapphireTest
         clearstatcache(); // needed to get accurate file_exists() results
 
         // Exception generated from including invalid file
-        $this->setExpectedException(
+        $this->expectException(InvalidArgumentException::class);
-            'InvalidArgumentException',
+        $this->expectExceptionMessage(sprintf(
-            sprintf(
+            "Requirements_Backend::combine_files(): Already included file(s) %s in combined file '%s'",
-                "Requirements_Backend::combine_files(): Already included file(s) %s in combined file '%s'",
+            $basePath . '/javascript/RequirementsTest_c.js',
-                $basePath . '/javascript/RequirementsTest_c.js',
+            'RequirementsTest_bc.js'
-                'RequirementsTest_bc.js'
+        ));
-            )
-        );
         $backend->combineFiles(
             'RequirementsTest_ac.js',
             array(
@@ -643,9 +622,7 @@ class RequirementsTest extends SapphireTest
     {
         $basePath = $this->getThemeRoot();
 
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
 
@@ -672,9 +649,7 @@ class RequirementsTest extends SapphireTest
     {
         $basePath = $this->getThemeRoot();
 
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
         $backend->javascript($basePath . '/a.js');
@@ -733,9 +708,7 @@ class RequirementsTest extends SapphireTest
     {
         $testPath = $this->getThemeRoot();
 
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
         $holder = Requirements::backend();
@@ -796,9 +769,7 @@ class RequirementsTest extends SapphireTest
 
     public function testJsWriteToBody()
     {
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
         $backend->javascript('http://www.mydomain.com/test.js');
@@ -813,15 +784,13 @@ class RequirementsTest extends SapphireTest
         $backend->setWriteJavascriptToBody(true);
         $html = $backend->includeInHTML($template);
         $this->assertNotContains('<head><script', $html);
-        $this->assertContains('</script></body>', $html);
+        $this->assertContains("</script>\n</body>", $html);
     }
 
     public function testIncludedJsIsNotCommentedOut()
     {
         $template = '<html><head></head><body><!--<script>alert("commented out");</script>--></body></html>';
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
         $backend->javascript($this->getThemeRoot() . '/javascript/RequirementsTest_a.js');
@@ -847,16 +816,15 @@ class RequirementsTest extends SapphireTest
         $html = $backend->includeInHTML($template);
         $this->assertEquals(
             '<html><head></head><body><!--<script>alert("commented out");</script>-->'
-            . '<h1>more content</h1><script type="application/javascript" src="' . $urlSrc . '"></script></body></html>',
+            . '<h1>more content</h1><script type="application/javascript" src="' . $urlSrc
+            . "\"></script>\n</body></html>",
             $html
         );
     }
 
     public function testForceJsToBottom()
     {
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
         $backend->javascript('http://www.mydomain.com/test.js');
@@ -872,10 +840,10 @@ EOS
         $template = '<html><head></head><body><header>My header</header><p>Body<script></script></p></body></html>';
 
         // The expected outputs
-        $expectedScripts = "<script type=\"application/javascript\" src=\"http://www.mydomain.com/test.js\">"
+        $expectedScripts = "<script type=\"application/javascript\" src=\"http://www.mydomain.com/test.js\"></script>\n"
-            . "</script><script type=\"application/javascript\">//<![CDATA[\n"
+            . "<script type=\"application/javascript\">//<![CDATA[\n"
             . "var globalvar = {\n\tpattern: '\\\\\$custom\\\\1'\n};\n"
-            . "//]]></script>";
+            . "//]]></script>\n";
         $JsInHead = "<html><head>$expectedScripts</head><body><header>My header</header><p>Body<script></script></p></body></html>";
         $JsInBody = "<html><head></head><body><header>My header</header><p>Body$expectedScripts<script></script></p></body></html>";
         $JsAtEnd  = "<html><head></head><body><header>My header</header><p>Body<script></script></p>$expectedScripts</body></html>";
@@ -922,9 +890,7 @@ EOS
         $template = '<html><head></head><body><header>My header</header><p>Body</p></body></html>';
         $basePath = $this->getThemeRoot();
 
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $backend = Injector::inst()->create(Requirements_Backend::class);
         $this->setupRequirements($backend);
 
@@ -954,9 +920,7 @@ EOS
      */
     public function testProvidedFiles()
     {
-        /**
+        /** @var Requirements_Backend $backend */
- * @var Requirements_Backend $backend
-*/
         $template = '<html><head></head><body><header>My header</header><p>Body</p></body></html>';
         $basePath = $this->getThemeRoot();