tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-09-30 05:09:06 +02:00
Code Issues Projects Releases Wiki Activity

FIX Do not insert requirements more than once in includeInHTML

Browse Source 
This change consolidates the string replacements used to insert requirements into the page content to help ensure
that they are not compounding and overwriting eachother.

The added test case includes where a user may have a Javascript snippet that contains a closing head tag, and the
test ensures that it does not get injected with requirements as well as the actual head tag in the DOM.
This commit is contained in:
Robbie Averill 2017-04-07 12:17:38 +12:00 committed by Sam Minnée
parent 2fd3b84b46
commit 55eb7ebdcc
2 changed files with 57 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
tests/forms/RequirementsTest.php
View File

@ -452,6 +452,36 @@ class RequirementsTest extends SapphireTest {
$this->assertNotRegexp('/RequirementsTest_b\.css\?m=[\d]*&foo=bar&bla=blubb"/', $html);
}
/**
* Ensure that if a JS snippet somewhere in the page (via requirements) contains </head> that it doesn't
* get injected with requirements
*/
public function testHeadTagIsNotInjectedTwice() {
$template = '<html><head></head><body><header>My header</header><p>Body</p></body></html>';
$basePath = $this->getCurrentRelativePath();
$backend = new Requirements_Backend;
// The offending snippet that contains a closing head tag
$backend->customScript('var myvar="<head></head>";');
// Insert a variety of random requirements
$backend->css($basePath .'/RequirementsTest_a.css');
$backend->javascript($basePath .'/RequirementsTest_a.js');
$backend->insertHeadTags('<link rel="alternate" type="application/atom+xml" title="test" href="/" />');
// Case A: JS forced to bottom
$backend->set_force_js_to_bottom(true);
$this->assertContains('var myvar="<head></head>";', $backend->includeInHTML(false, $template));
// Case B: JS written to body
$backend->set_force_js_to_bottom(false);
$backend->set_write_js_to_body(true);
$this->assertContains('var myvar="<head></head>";', $backend->includeInHTML(false, $template));
// Case C: Neither of the above
$backend->set_write_js_to_body(false);
$this->assertContains('var myvar="<head></head>";', $backend->includeInHTML(false, $template));
}
public function assertFileIncluded($backend, $type, $files) {
$type = strtolower($type);
switch (strtolower($type)) {

40
view/Requirements.php
View File

@ -863,19 +863,18 @@ class Requirements_Backend {
$requirements .= "$customHeadTag\n";
}
$replacements = array();
if ($this->force_js_to_bottom) {
// Remove all newlines from code to preserve layout
$jsRequirements = preg_replace('/>\n*/', '>', $jsRequirements);
$jsRequirements = $this->removeNewlinesFromCode($jsRequirements);
// Forcefully put the scripts at the bottom of the body instead of before the first
// script tag.
$content = preg_replace("/(<\/body[^>]*>)/i", $jsRequirements . "\\1", $content);
$replacements["/(<\/body[^>]*>)/i"] = $jsRequirements . "\\1";
// Put CSS at the bottom of the head
$content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content);
} elseif($this->write_js_to_body) {
// Remove all newlines from code to preserve layout
$jsRequirements = preg_replace('/>\n*/', '>', $jsRequirements);
$replacements["/(<\/head>)/i"] = $requirements . "\\1";
} elseif ($this->write_js_to_body) {
$jsRequirements = $this->removeNewlinesFromCode($jsRequirements);
// If your template already has script tags in the body, then we try to put our script
// tags just before those. Otherwise, we put it at the bottom.
@ -892,23 +891,38 @@ class Requirements_Backend {
$commentTags[1] == '-->'
);
if($canWriteToBody) {
$content = substr($content,0,$p1) . $jsRequirements . substr($content,$p1);
if ($canWriteToBody) {
$content = substr($content, 0, $p1) . $jsRequirements . substr($content, $p1);
} else {
$content = preg_replace("/(<\/body[^>]*>)/i", $jsRequirements . "\\1", $content);
$replacements["/(<\/body[^>]*>)/i"] = $jsRequirements . "\\1";
}
// Put CSS at the bottom of the head
$content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content);
$replacements["/(<\/head>)/i"] = $requirements . "\\1";
} else {
$content = preg_replace("/(<\/head>)/i", $requirements . "\\1", $content);
$content = preg_replace("/(<\/head>)/i", $jsRequirements . "\\1", $content);
// Put CSS and Javascript together before the closing head tag
$replacements["/(<\/head>)/i"] = $requirements . $jsRequirements. "\\1";
}
if (!empty($replacements)) {
// Replace everything at once (only once)
$content = preg_replace(array_keys($replacements), array_values($replacements), $content, 1);
}
}
return $content;
}
/**
* Remove all newlines from code to preserve layout
*
* @param string $code
* @return string
*/
protected function removeNewlinesFromCode($code) {
return preg_replace('/>\n*/', '>', $code);
}
/**
* Attach requirements inclusion to X-Include-JS and X-Include-CSS headers on the given
* HTTP Response