ENGCOM-7154: Adjust mechanism that moves all scripts to the end of the page #27270

slavvka · web-flow · commit 4db9f5f2e8c1 · 2020-05-19T01:49:56.000-05:00
diff --git a/app/code/Magento/Theme/Controller/Result/JsFooterPlugin.php b/app/code/Magento/Theme/Controller/Result/JsFooterPlugin.php
@@ -8,11 +8,13 @@
 namespace Magento\Theme\Controller\Result;
 
 use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\App\Response\HttpInterface as HttpResponseInterface;
+use Magento\Framework\App\ResponseInterface;
+use Magento\Framework\View\Result\Layout;
 use Magento\Store\Model\ScopeInterface;
-use Magento\Framework\App\Response\Http;
 
 /**
- * Plugin for putting all js to footer.
+ * Plugin for putting all JavaScript tags to the end of body.
  */
 class JsFooterPlugin
 {
@@ -32,34 +34,77 @@ public function __construct(ScopeConfigInterface $scopeConfig)
     }
 
     /**
-     * Put all javascript to footer before sending the response.
+     * Moves all JavaScript tags to the end of body if this feature is enabled.
      *
-     * @param Http $subject
-     * @return void
+     * @param Layout $subject
+     * @param Layout $result
+     * @param HttpResponseInterface|ResponseInterface $httpResponse
+     * @return Layout (That should be void, actually)
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
-    public function beforeSendResponse(Http $subject)
+    public function afterRenderResult(Layout $subject, Layout $result, ResponseInterface $httpResponse)
     {
-        $content = $subject->getContent();
-        $script = [];
-        if (is_string($content) && strpos($content, '</body') !== false) {
-            if ($this->scopeConfig->isSetFlag(
-                self::XML_PATH_DEV_MOVE_JS_TO_BOTTOM,
-                ScopeInterface::SCOPE_STORE
-            )
-            ) {
-                $pattern = '#<script[^>]*+(?<!text/x-magento-template.)>.*?</script>#is';
-                $content = preg_replace_callback(
-                    $pattern,
-                    function ($matchPart) use (&$script) {
-                        $script[] = $matchPart[0];
-                        return '';
-                    },
-                    $content
-                );
-                $subject->setContent(
-                    str_replace('</body', implode("\n", $script) . "\n</body", $content)
-                );
+        if (!$this->isDeferEnabled()) {
+            return $result;
+        }
+
+        $content = (string)$httpResponse->getContent();
+        $bodyEndTag = '</body';
+        $bodyEndTagFound = strrpos($content, $bodyEndTag) !== false;
+
+        if ($bodyEndTagFound) {
+            $scripts = $this->extractScriptTags($content);
+            if ($scripts) {
+                $newBodyEndTagPosition = strrpos($content, $bodyEndTag);
+                $content = substr_replace($content, $scripts . "\n", $newBodyEndTagPosition, 0);
+                $httpResponse->setContent($content);
+            }
+        }
+
+        return $result;
+    }
+
+    /**
+     * Extracts and returns script tags found in given content.
+     *
+     * @param string $content
+     */
+    private function extractScriptTags(&$content): string
+    {
+        $scripts = '';
+        $scriptOpen = '<script';
+        $scriptClose = '</script>';
+        $scriptOpenPos = strpos($content, $scriptOpen);
+
+        while ($scriptOpenPos !== false) {
+            $scriptClosePos = strpos($content, $scriptClose, $scriptOpenPos);
+            $script = substr($content, $scriptOpenPos, $scriptClosePos - $scriptOpenPos + strlen($scriptClose));
+            $isXMagentoTemplate = strpos($script, 'text/x-magento-template') !== false;
+
+            if ($isXMagentoTemplate) {
+                $scriptOpenPos = strpos($content, $scriptOpen, $scriptClosePos);
+                continue;
             }
+
+            $scripts .= "\n" . $script;
+            $content = str_replace($script, '', $content);
+            // Script cut out, continue search from its position.
+            $scriptOpenPos = strpos($content, $scriptOpen, $scriptOpenPos);
         }
+
+        return $scripts;
+    }
+
+    /**
+     * Returns information whether moving JS to footer is enabled
+     *
+     * @return bool
+     */
+    private function isDeferEnabled(): bool
+    {
+        return $this->scopeConfig->isSetFlag(
+            self::XML_PATH_DEV_MOVE_JS_TO_BOTTOM,
+            ScopeInterface::SCOPE_STORE
+        );
     }
 }
diff --git a/app/code/Magento/Theme/Test/Unit/Controller/Result/JsFooterPluginTest.php b/app/code/Magento/Theme/Test/Unit/Controller/Result/JsFooterPluginTest.php
@@ -10,6 +10,7 @@
 use Magento\Framework\App\Config\ScopeConfigInterface;
 use Magento\Framework\App\Response\Http;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
+use Magento\Framework\View\Result\Layout;
 use Magento\Store\Model\ScopeInterface;
 use Magento\Theme\Controller\Result\JsFooterPlugin;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -22,21 +23,18 @@ class JsFooterPluginTest extends TestCase
 {
     const STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM = 'dev/js/move_script_to_bottom';
 
-    /**
-     * @var JsFooterPlugin
-     */
+    /** @var JsFooterPlugin */
     private $plugin;
 
-    /**
-     * @var ScopeConfigInterface|MockObject
-     */
+    /** @var ScopeConfigInterface|MockObject */
     private $scopeConfigMock;
 
-    /**
-     * @var Http|MockObject
-     */
+    /** @var Http|MockObject */
     private $httpMock;
 
+    /** @var Layout|MockObject */
+    private $layoutMock;
+
     /**
      * @inheritdoc
      */
@@ -48,6 +46,7 @@ protected function setUp(): void
             ->getMockForAbstractClass();
 
         $this->httpMock = $this->createMock(Http::class);
+        $this->layoutMock = $this->createMock(Layout::class);
 
         $objectManager = new ObjectManagerHelper($this);
         $this->plugin = $objectManager->getObject(
@@ -59,11 +58,11 @@ protected function setUp(): void
     }
 
     /**
-     * Data Provider for testBeforeSendResponse()
+     * Data Provider for testAfterRenderResult()
      *
      * @return array
      */
-    public function sendResponseDataProvider(): array
+    public function renderResultDataProvider(): array
     {
         return [
             'content_with_script_tag' => [
@@ -74,9 +73,9 @@ public function sendResponseDataProvider(): array
                 "flag" => true,
                 "result" => "<body><h1>Test Title</h1>" .
                     "<script type=\"text/x-magento-template\">test</script>" .
-                    "<p>Test Content</p>" .
-                    "<script type=\"text/x-magento-init\">test</script>" .
-                    "\n</body>"
+                    "<p>Test Content</p>\n" .
+                    "<script type=\"text/x-magento-init\">test</script>\n" .
+                    "</body>"
             ],
             'content_with_config_disable' => [
                 "content" => "<body><p>Test Content</p></body>",
@@ -98,67 +97,61 @@ public function sendResponseDataProvider(): array
      * @param bool $isSetFlag
      * @param string $result
      * @return void
-     * @dataProvider sendResponseDataProvider
+     * @dataProvider renderResultDataProvider
      */
-    public function testBeforeSendResponse($content, $isSetFlag, $result): void
+    public function testAfterRenderResult($content, $isSetFlag, $result): void
     {
-        $this->httpMock->expects($this->once())
-            ->method('getContent')
+        // Given (context)
+        $this->httpMock->method('getContent')
             ->willReturn($content);
 
-        $this->scopeConfigMock->expects($this->once())
-            ->method('isSetFlag')
-            ->with(
-                self::STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM,
-                ScopeInterface::SCOPE_STORE
-            )
+        $this->scopeConfigMock->method('isSetFlag')
+            ->with(self::STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM, ScopeInterface::SCOPE_STORE)
             ->willReturn($isSetFlag);
 
+        // Expects
         $this->httpMock->expects($this->any())
             ->method('setContent')
             ->with($result);
 
-        $this->plugin->beforeSendResponse($this->httpMock);
+        // When
+        $this->plugin->afterRenderResult($this->layoutMock, $this->layoutMock, $this->httpMock);
     }
 
     /**
-     * Data Provider for testBeforeSendResponseIfGetContentIsNotAString()
+     * Data Provider for testAfterRenderResultIfGetContentIsNotAString()
      *
      * @return array
      */
     public function ifGetContentIsNotAStringDataProvider(): array
     {
         return [
-            'empty_array' => [
-                'content' => []
-            ],
             'null' => [
                 'content' => null
             ]
         ];
     }
 
     /**
-     * Test BeforeSendResponse if content is not a string
+     * Test AfterRenderResult if content is not a string
      *
      * @param string $content
      * @return void
      * @dataProvider ifGetContentIsNotAStringDataProvider
      */
-    public function testBeforeSendResponseIfGetContentIsNotAString($content): void
+    public function testAfterRenderResultIfGetContentIsNotAString($content): void
     {
+        $this->scopeConfigMock->method('isSetFlag')
+            ->with(self::STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM, ScopeInterface::SCOPE_STORE)
+            ->willReturn(true);
+
         $this->httpMock->expects($this->once())
             ->method('getContent')
             ->willReturn($content);
 
-        $this->scopeConfigMock->expects($this->never())
-            ->method('isSetFlag')
-            ->with(
-                self::STUB_XML_PATH_DEV_MOVE_JS_TO_BOTTOM,
-                ScopeInterface::SCOPE_STORE
-            )
-            ->willReturn(false);
+        $this->httpMock->expects($this->never())
+            ->method('setContent');
 
-        $this->plugin->beforeSendResponse($this->httpMock);
+        $this->plugin->afterRenderResult($this->layoutMock, $this->layoutMock, $this->httpMock);
     }
 }
diff --git a/app/code/Magento/Theme/etc/frontend/di.xml b/app/code/Magento/Theme/etc/frontend/di.xml
@@ -27,9 +27,11 @@
         <plugin name="result-messages" type="Magento\Theme\Controller\Result\MessagePlugin"/>
     </type>
     <type name="Magento\Framework\App\Response\Http">
-        <plugin name="result-js-footer" type="Magento\Theme\Controller\Result\JsFooterPlugin"/>
         <plugin name="asyncCssLoad" type="Magento\Theme\Controller\Result\AsyncCssPlugin"/>
     </type>
+    <type name="Magento\Framework\View\Result\Layout">
+        <plugin name="deferJsToFooter" type="Magento\Theme\Controller\Result\JsFooterPlugin" sortOrder="-10"/>
+    </type>
     <type name="Magento\Theme\Block\Html\Header\CriticalCss">
         <arguments>
             <argument name="filePath" xsi:type="string">css/critical.css</argument>
