Commit 6f6279fd authored by Luke Leber's avatar Luke Leber
Browse files

Issue #3260744 by Luke.Leber, proweb.ua: Empty style tags may be erroneously injected into markup

parent 7b8e85a1

src/Asset/CriticalCssCollectionRenderer.php

+10 −4
Original line number Diff line number Diff line
@@ -131,13 +131,13 @@ class CriticalCssCollectionRenderer implements AssetCollectionRendererInterface 
    $event = new CssPreRenderEvent($css);

    /* @noinspection PhpMethodParametersCountMismatchInspection */

    $this->eventDispatcher->dispatch($event, CssPreRenderEvent::EVENT_NAME);

    $css = $event->getCss();

    $css = trim($event->getCss());



    return [

    return $css ? [

      '#type' => 'html_tag',

      '#tag' => 'style',

      '#value' => Markup::create($css),

    ];

    ] : NULL;

  }



  /**
@@ -150,7 +150,13 @@ class CriticalCssCollectionRenderer implements AssetCollectionRendererInterface 
      $enabled_themes = $this->config->get('enabled_themes');

      $active_theme = $this->themeManager->getActiveTheme()->getName();

      if (empty($enabled_themes) || in_array($active_theme, $enabled_themes, TRUE)) {

        $elements[] = $this->getInlineCss($assets);

        $inline_css = $this->getInlineCss($assets);

        if ($inline_css) {

          $elements[] = $inline_css;

        }

        else {

          $elements = $this->cssCollectionRenderer->render($assets);

        }

      }

      else {

        $elements = $this->cssCollectionRenderer->render($assets);

tests/src/Kernel/CssCollectionRendererTest.php

+44 −14
Original line number Diff line number Diff line
@@ -61,6 +61,13 @@ class CssCollectionRendererTest extends KernelTestBase { 
   */

  protected $config;



  /**

   * The mock event dispatcher.

   *

   * @var \PHPUnit\Framework\MockObject\MockObject|EventDispatcherInterface

   */

  protected $eventDispatcher;



  /**

   * {@inheritdoc}

   */
@@ -78,21 +85,10 @@ class CssCollectionRendererTest extends KernelTestBase { 
        ['public://test-3.css', __DIR__ . '/../../fixtures/test-3.css'],

      ]);



    $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)

    $this->eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)

      ->disableOriginalConstructor()

      ->getMock();



    $eventDispatcher->method('dispatch')

      ->willReturnCallback(static function (CssPreRenderEvent $event) {

        $css = $event->getCss();

        $css .= <<<CSS

p {

  color: gray;

}

CSS;

        $event->setCss($css);

      });



    $this->cssCollectionRenderer = $this->getMockBuilder(AssetCollectionRendererInterface::class)

      ->disableOriginalConstructor()

      ->getMock();
@@ -155,7 +151,7 @@ CSS 
      ->method('get')

      ->willReturn($logger);



    $this->instance = new CriticalCssCollectionRenderer($this->cssCollectionRenderer, $config_factory, $this->themeManager, $filesystem, $eventDispatcher, $http_client, $logger_factory);

    $this->instance = new CriticalCssCollectionRenderer($this->cssCollectionRenderer, $config_factory, $this->themeManager, $filesystem, $this->eventDispatcher, $http_client, $logger_factory);

  }



  /**
@@ -175,6 +171,25 @@ CSS 
      ]);

  }



  /**

   * Sets up the CSS to add in the dispatched event.

   *

   * @param string $css

   *   The CSS to add.

   */

  protected function setEventCss() {

    $this->eventDispatcher->method('dispatch')

      ->willReturnCallback(static function (CssPreRenderEvent $event) {

        $css = $event->getCss();

        $css .= <<<CSS

p {

  color: gray;

}

CSS;

        $event->setCss($css);

      });

  }



  /**

   * Sets the current active theme.

   *
@@ -254,6 +269,8 @@ CSS 
      ->expects(static::never())

      ->method('render');



    $this->setEventCss();



    $expected = [

      [

        '#type' => 'html_tag',
@@ -278,8 +295,21 @@ CSS), 
      ],

    ];

    $actual = $this->instance->render(static::ASSETS);



    static::assertEquals($expected, $actual);

  }



  /**

   * Test case for when there are no assets (some big_pipe responses).

   */

  public function testCssCollectionRendererNoAssets() {

    $this->setConfig(TRUE, []);



    $this->setActiveTheme('should_not_matter');

    $this->cssCollectionRenderer

      ->method('render')

      ->willReturn([]);



    static::assertSame([], $this->instance->render([]));

  }



}