Skip to content
Permalink
Browse files

FIX Do not insert requirements more than once in includeInHTML

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.
  • Loading branch information...
robbieaverill authored and sminnee committed Apr 7, 2017
1 parent 2fd3b84 commit 55eb7ebdcc9ba767f978dff510614bbd2e0c309d
Showing with 57 additions and 13 deletions.
  1. +30 −0 tests/forms/RequirementsTest.php
  2. +27 −13 view/Requirements.php
@@ -452,6 +452,36 @@ public function testSuffix() {
$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)) {
@@ -863,19 +863,18 @@ public function includeInHTML($templateFile, $content) {
$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 @@ public function includeInHTML($templateFile, $content) {
$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

0 comments on commit 55eb7eb

Please sign in to comment.
You can’t perform that action at this time.