From 64e802f7955df2738d6d7381d30843f0cd438332 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 18 Apr 2017 10:34:18 +1200 Subject: [PATCH] API Move createTag to HTML class ENHANCEMENT Better HTML generation behaviour for Requirements_Backend --- _config/html.yml | 4 +- docs/en/04_Changelogs/4.0.0.md | 3 + src/Forms/FormField.php | 55 ---------- src/Forms/GridField/GridField.php | 26 +++-- src/Forms/GridField/GridFieldLevelup.php | 4 +- .../HTMLEditor/EmbedShortcodeProvider.php | 8 +- src/Forms/HTMLEditor/HTMLEditorField.php | 4 +- src/Forms/SelectionGroup.php | 4 +- src/ORM/FieldType/DBHTMLText.php | 3 +- src/View/HTML.php | 78 ++++++++++++++ src/View/Parsers/Diff.php | 2 +- src/View/Parsers/PurifierHTMLCleaner.php | 3 +- src/View/Parsers/ShortcodeParser.php | 4 +- src/View/Requirements_Backend.php | 53 ++++++---- tests/behat/features/login.feature | 2 +- tests/behat/features/lostpassword.feature | 2 +- tests/behat/features/manage-users.feature | 2 +- tests/behat/features/profile.feature | 1 + .../features/security-permissions.feature | 2 +- tests/php/Forms/FormFieldTest.php | 12 --- .../HTMLEditor/HTMLEditorSanitiserTest.php | 3 +- tests/php/View/HTMLTest.php | 58 ++++++++++ tests/php/View/RequirementsTest.php | 100 ++++++------------ 23 files changed, 244 insertions(+), 189 deletions(-) create mode 100644 src/View/HTML.php create mode 100644 tests/php/View/HTMLTest.php diff --git a/_config/html.yml b/_config/html.yml index 42f02705b..0c3d04c2f 100644 --- a/_config/html.yml +++ b/_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 diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 1d877e160..92ce5e0f4 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/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: diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index 93e2e488f..ce792983b 100644 --- a/src/Forms/FormField.php +++ b/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', - $tag, - $preparedAttributes, - $content, - $tag - ); - } - - return sprintf( - '<%s%s />', - $tag, - $preparedAttributes - ); - } - /** * Creates a new field. * diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index b4f1a6e32..1c442f3da 100644 --- a/src/Forms/GridField/GridField.php +++ b/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|void - * + * @return array|RequestHandler|HTTPResponse|string * @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'] diff --git a/src/Forms/GridField/GridFieldLevelup.php b/src/Forms/GridField/GridFieldLevelup.php index 807e3a193..d16d41a88 100644 --- a/src/Forms/GridField/GridFieldLevelup.php +++ b/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) diff --git a/src/Forms/HTMLEditor/EmbedShortcodeProvider.php b/src/Forms/HTMLEditor/EmbedShortcodeProvider.php index 78cee9c28..b27cae492 100644 --- a/src/Forms/HTMLEditor/EmbedShortcodeProvider.php +++ b/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); } } diff --git a/src/Forms/HTMLEditor/HTMLEditorField.php b/src/Forms/HTMLEditor/HTMLEditorField.php index a46a01086..846e7f074 100644 --- a/src/Forms/HTMLEditor/HTMLEditorField.php +++ b/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); diff --git a/src/Forms/SelectionGroup.php b/src/Forms/SelectionGroup.php index 81a5c309f..81b1bb3b8 100644 --- a/src/Forms/SelectionGroup.php +++ b/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', diff --git a/src/ORM/FieldType/DBHTMLText.php b/src/ORM/FieldType/DBHTMLText.php index c3624a701..2219eec3b 100644 --- a/src/ORM/FieldType/DBHTMLText.php +++ b/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()'; diff --git a/src/View/HTML.php b/src/View/HTML.php new file mode 100644 index 000000000..220d0e955 --- /dev/null +++ b/src/View/HTML.php @@ -0,0 +1,78 @@ + $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}"; + } +} diff --git a/src/View/Parsers/Diff.php b/src/View/Parsers/Diff.php index 73b6a105d..c32606fc3 100644 --- a/src/View/Parsers/Diff.php +++ b/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(); } diff --git a/src/View/Parsers/PurifierHTMLCleaner.php b/src/View/Parsers/PurifierHTMLCleaner.php index 1c909c033..772bd22d7 100644 --- a/src/View/Parsers/PurifierHTMLCleaner.php +++ b/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(); } } diff --git a/src/View/Parsers/ShortcodeParser.php b/src/View/Parsers/ShortcodeParser.php index edcf81f28..411727699 100644 --- a/src/View/Parsers/ShortcodeParser.php +++ b/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; } diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index 602c1f53b..16337d938 100644 --- a/src/View/Requirements_Backend.php +++ b/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" : ""; - $defer = (isset($attributes['defer']) && $attributes['defer'] == true) ? " defer" : ""; - $type = Convert::raw2att(isset($attributes['type']) ? $attributes['type'] : "application/javascript"); - $path = Convert::raw2att($this->pathForFile($file)); - if ($path) { - $jsRequirements .= ""; + // Build html attributes + $htmlAttributes = [ + 'type' => isset($attributes['type']) ? $attributes['type'] : "application/javascript", + 'src' => $this->pathForFile($file), + ]; + 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 .= ""; + $jsRequirements .= HTML::createTag( + 'script', + [ 'type' => 'application/javascript' ], + "//" + ); + $jsRequirements .= "\n"; } + // CSS file links foreach ($this->getCSS() as $file => $params) { - $path = Convert::raw2att($this->pathForFile($file)); - if ($path) { - $media = (isset($params['media']) && !empty($params['media'])) - ? " media=\"{$params['media']}\"" : ""; - $requirements .= "\n"; + $htmlAttributes = [ + 'rel' => 'stylesheet', + 'type' => 'text/css', + 'href' => $this->pathForFile($file), + ]; + 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 .= "\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::config()->default_locale . '.js', + i18n::getData()->langFromLocale(i18n::config()->get('default_locale')) . '.js', + i18n::config()->get('default_locale') . '.js', i18n::getData()->langFromLocale(i18n::get_locale()) . '.js', i18n::get_locale() . '.js', ); diff --git a/tests/behat/features/login.feature b/tests/behat/features/login.feature index 577b20c25..5dbaf8f95 100644 --- a/tests/behat/features/login.feature +++ b/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 diff --git a/tests/behat/features/lostpassword.feature b/tests/behat/features/lostpassword.feature index bcfe408bf..ddbf86218 100644 --- a/tests/behat/features/lostpassword.feature +++ b/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 diff --git a/tests/behat/features/manage-users.feature b/tests/behat/features/manage-users.feature index a264b3653..58d689c61 100644 --- a/tests/behat/features/manage-users.feature +++ b/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 diff --git a/tests/behat/features/profile.feature b/tests/behat/features/profile.feature index a13c02c74..8c21656ec 100644 --- a/tests/behat/features/profile.feature +++ b/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 diff --git a/tests/behat/features/security-permissions.feature b/tests/behat/features/security-permissions.feature index 9fc3ec8ec..ded5cb0f8 100644 --- a/tests/behat/features/security-permissions.feature +++ b/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 diff --git a/tests/php/Forms/FormFieldTest.php b/tests/php/Forms/FormFieldTest.php index 329313a25..ea883a909 100644 --- a/tests/php/Forms/FormFieldTest.php +++ b/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('', $tag); - $this->assertRegexp('#/>$#', $tag); - } } diff --git a/tests/php/Forms/HTMLEditor/HTMLEditorSanitiserTest.php b/tests/php/Forms/HTMLEditor/HTMLEditorSanitiserTest.php index e0f69caa3..9716d965c 100644 --- a/tests/php/Forms/HTMLEditor/HTMLEditorSanitiserTest.php +++ b/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); diff --git a/tests/php/View/HTMLTest.php b/tests/php/View/HTMLTest.php new file mode 100644 index 000000000..e363e3e1a --- /dev/null +++ b/tests/php/View/HTMLTest.php @@ -0,0 +1,58 @@ + 'description', + 'content' => 'test tag', + ]); + $this->assertEquals('', $tag); + } + + public function testEmptyAttributes() + { + $tag = HTML::createTag('meta', [ + 'value' => 0, + 'content' => '', + 'max' => 3, + 'details' => null, + 'disabled' => false, + 'readonly' => true, + ]); + $this->assertEquals('', $tag); + } + + public function testNormalTag() + { + $tag = HTML::createTag('a', [ + 'title' => 'Some link', + 'nullattr' => null, + ]); + $this->assertEquals('', $tag); + + $tag = HTML::createTag('a', [ + 'title' => 'HTML & Text', + 'nullattr' => null, + ], 'Some content!'); + $this->assertEquals('Some content!', $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 content!'); + } +} diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index 85da1a3de..806320b94 100644 --- a/tests/php/View/RequirementsTest.php +++ b/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( - 'InvalidArgumentException', - sprintf( - "Requirements_Backend::combine_files(): Already included file(s) %s in combined file '%s'", - $basePath . '/javascript/RequirementsTest_c.js', - 'RequirementsTest_bc.js' - ) - ); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf( + "Requirements_Backend::combine_files(): Already included file(s) %s in combined file '%s'", + $basePath . '/javascript/RequirementsTest_c.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('assertContains('', $html); + $this->assertContains("\n", $html); } public function testIncludedJsIsNotCommentedOut() { $template = ''; - /** - * @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( '' - . '

more content

', + . '

more content

\n", $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 = '
My header

Body

'; // The expected outputs - $expectedScripts = "\n" + . ""; + . "//]]>\n"; $JsInHead = "$expectedScripts
My header

Body

"; $JsInBody = "
My header

Body$expectedScripts

"; $JsAtEnd = "
My header

Body

$expectedScripts"; @@ -922,9 +890,7 @@ EOS $template = '
My header

Body

'; $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 = '
My header

Body

'; $basePath = $this->getThemeRoot();