Unverified Commit c8c8cb5c authored by Alex Pott's avatar Alex Pott
Browse files

fix: #3516264 CKEditor 5 loads all plugin translations on AJAX operations 

By: twod
By: smustgrave
By: catch
By: godotislate
By: alexpott
parent cc433fe1

core/modules/ckeditor5/src/Hook/Ckeditor5Hooks.php

+28 −17
Original line number Diff line number Diff line
@@ -2,6 +2,8 @@ 


namespace Drupal\ckeditor5\Hook;



use Drupal\Core\Asset\LibraryDependencyResolverInterface;

use Drupal\Core\Extension\ModuleHandlerInterface;

use Drupal\Core\Field\FieldItemListInterface;

use Drupal\ckeditor5\LanguageMapper;

use Drupal\Core\Hook\Order\OrderAfter;
@@ -26,6 +28,8 @@ class Ckeditor5Hooks { 


  public function __construct(

    protected LanguageMapper $languageMapper,

    protected ModuleHandlerInterface $moduleHandler,

    protected LibraryDependencyResolverInterface $libraryDependencyResolver,

  ) {



  }
@@ -221,7 +225,6 @@ public function libraryInfoAlter(&$libraries, $extension): void { 
    if ($extension === 'filter') {

      $libraries['drupal.filter.admin']['dependencies'][] = 'ckeditor5/internal.drupal.ckeditor5.filter.admin';

    }

    $moduleHandler = \Drupal::moduleHandler();

    if ($extension === 'ckeditor5') {

      // Add paths to stylesheets specified by a theme's ckeditor5-stylesheets

      // config property.
@@ -238,7 +241,7 @@ public function libraryInfoAlter(&$libraries, $extension): void { 
      $libraries['drupal.dialog']['js']['modules/ckeditor5/js/ckeditor5.dialog.fix.js'] = [];

    }

    // Only add translation processing if the locale module is enabled.

    if (!$moduleHandler->moduleExists('locale')) {

    if (!$this->moduleHandler->moduleExists('locale')) {

      return;

    }

    // All possibles CKEditor 5 languages that can be used by Drupal.
@@ -266,7 +269,7 @@ public function libraryInfoAlter(&$libraries, $extension): void { 
      $path = 'core';

    }

    else {

      if ($moduleHandler->moduleExists($extension)) {

      if ($this->moduleHandler->moduleExists($extension)) {

        $extension_type = 'module';

      }

      else {
@@ -322,25 +325,31 @@ public function libraryInfoAlter(&$libraries, $extension): void { 
   */

  #[Hook('js_alter')]

  public function jsAlter(&$javascript, AttachedAssetsInterface $assets, LanguageInterface $language): void {

    // This file means CKEditor 5 translations are in use on the page.

    // @see locale_js_alter()

    $placeholder_file = 'core/assets/vendor/ckeditor5/translation.js';

    // This file is used to get a weight that will make it possible to aggregate

    // all translation files in a single aggregate.

    // When the locale module isn't installed there are no translations.

    if (!$this->moduleHandler->moduleExists('locale')) {

      unset($javascript[$placeholder_file]);

      return;

    }

    $translations_library = 'core/ckeditor5.translations';

    if (in_array($translations_library, $this->libraryDependencyResolver->getLibrariesWithDependencies($assets->getLibraries()), TRUE)

      || in_array($translations_library, $this->libraryDependencyResolver->getLibrariesWithDependencies($assets->getAlreadyLoadedLibraries()), TRUE)) {



      // This file is used to get a weight that will make it possible to

      // aggregate all translation files in a single aggregate.

      $ckeditor_dll_file = 'core/assets/vendor/ckeditor5/ckeditor5-dll/ckeditor5-dll.js';

    if (isset($javascript[$placeholder_file])) {

      // Use the placeholder file weight to set all the translations files

      // weights so they can be aggregated together as expected.

      $default_weight = $javascript[$placeholder_file]['weight'];

      // weights so they can be aggregated together as expected. Account for

      // requests where the library is not loaded such as when during an AJAX

      // request when it was already loaded via the main request. In these cases

      // it is unlikely that multiple JavaScript aggregates will be created

      // anyway since AJAX requests generally result in very few libraries being

      // loaded.

      $default_weight = $javascript[$placeholder_file]['weight'] ?? 0;

      if (isset($javascript[$ckeditor_dll_file])) {

        $default_weight = $javascript[$ckeditor_dll_file]['weight'];

      }

      // The placeholder file is not a real file, remove it from the list.

      unset($javascript[$placeholder_file]);

      // When the locale module isn't installed there are no translations.

      if (!\Drupal::moduleHandler()->moduleExists('locale')) {

        return;

      }



      $ckeditor5_language = $this->languageMapper->getMapping($language->getId());

      // Remove all CKEditor 5 translations files that are not in the current

      // language.
@@ -361,6 +370,8 @@ public function jsAlter(&$javascript, AttachedAssetsInterface $assets, LanguageI 
        }

      }

    }

    // The placeholder file is not a real file, remove it from the list.

    unset($javascript[$placeholder_file]);

  }



  /**

core/modules/ckeditor5/tests/src/Unit/CKEditor5Test.php

+84 −0
Original line number Diff line number Diff line
@@ -4,7 +4,14 @@ 


namespace Drupal\Tests\ckeditor5\Unit;



use Drupal\ckeditor5\LanguageMapper;

use Drupal\ckeditor5\Hook\Ckeditor5Hooks;

use Drupal\ckeditor5\Plugin\Editor\CKEditor5;

use Drupal\Core\Asset\AttachedAssets;

use Drupal\Core\Asset\LibraryDependencyResolver;

use Drupal\Core\DependencyInjection\ContainerBuilder;

use Drupal\Core\Extension\ModuleHandlerInterface;

use Drupal\Core\Language\Language;

use Drupal\Tests\ckeditor5\Traits\PrivateMethodUnitTestTrait;

use Drupal\Tests\UnitTestCase;

use PHPUnit\Framework\Attributes\CoversClass;
@@ -108,4 +115,81 @@ public static function providerPathsToFormNames(): array { 
    ];

  }



  /**

   * Test the js_alter hook alters when expected.

   *

   * @legacy-covers \Drupal\ckeditor5\Hook\Ckeditor5Hooks::jsAlter

   */

  public function testJsAlterHook(): void {

    $placeholder_file = 'core/assets/vendor/ckeditor5/translation.js';

    $language_mapper = $this->createMock(LanguageMapper::class);

    $language_mapper->expects($this->any())

      ->method('getMapping')

      ->willReturn('en');

    $module_handler = $this->prophesize(ModuleHandlerInterface::class);

    $module_handler->moduleExists('locale')->willReturn(TRUE);

    $library_resolver = $this->createMock(LibraryDependencyResolver::class);

    $library_resolver->expects($this->any())

      ->method('getLibrariesWithDependencies')

      ->willReturn(['core/ckeditor5.translations.en', 'core/ckeditor5.translations', 'core/ckeditor5.anything']);

    $hooks = new Ckeditor5Hooks($language_mapper, $module_handler->reveal(), $library_resolver);

    $assets = new AttachedAssets();

    $assets->setLibraries([

      'core/ckeditor5.translations.en',

      'core/ckeditor5.anything',

    ]);

    $language = new Language([

      'id' => 'en',

      'name' => 'English',

      'direction' => 'ltr',

    ]);

    $original_javascript = [

      'keep_this' => [

        'ckeditor5_langcode' => 'en',

      ],

      'remove_this' => [

        'ckeditor5_langcode' => 'sv',

      ],

      'keep_this_too' => [],

    ];

    $expected_javascript = [

      'keep_this' => [

        'ckeditor5_langcode' => 'en',

        'weight' => 5,

      ],

      'keep_this_too' => [],

    ];



    $container = new ContainerBuilder();

    $module_handler = $this->prophesize(ModuleHandlerInterface::class);

    $module_handler->moduleExists('locale')->willReturn(TRUE);

    $container->set('module_handler', $module_handler->reveal());

    \Drupal::setContainer($container);



    // First check that it filters when the placeholder script is present.

    $javascript = $original_javascript + [

      $placeholder_file => [

        'weight' => 5,

      ],

    ];

    $hooks->jsAlter($javascript, $assets, $language);

    $this->assertEquals($expected_javascript, $javascript);



    // Next check it still filters if the placeholder script has already been

    // loaded and is now excluded from the list, such as an AJAX operation

    // loading a new format which uses another set of plugins.

    $assets = new AttachedAssets();

    $assets->setLibraries([

      'core/ckeditor5.translations.en',

    ]);

    $assets->setAlreadyLoadedLibraries([

      'core/ckeditor5.translations.anything',

    ]);

    $javascript = $original_javascript;

    $hooks->jsAlter($javascript, $assets, $language);

    // There was no placeholder to get the weight from.

    $expected_javascript['keep_this']['weight'] = 0;

    $this->assertEquals($expected_javascript, $javascript);

  }



}

core/profiles/demo_umami/tests/src/FunctionalJavascript/AssetAggregationAcrossPagesTest.php

+29 −1
Original line number Diff line number Diff line
@@ -73,7 +73,35 @@ public function testFrontAndRecipesPagesEditor(): void { 
      'ScriptCount' => 5,

      'ScriptBytes' => 262800,

      'StylesheetCount' => 5,

      'StylesheetBytes' => 203850,

      'StylesheetBytes' => 203800,

    ];

    $this->assertMetrics($expected, $performance_data);

  }



  /**

   * Checks the node/add page asset requests as an author.

   */

  public function testNodeAddPagesAuthor(): void {

    $user = $this->createUser();

    $user->addRole('author');

    $user->save();

    $this->drupalLogin($user);

    $this->drupalGet('<front>');

    // Give additional time for the request and all assets to be returned

    // before making the next request.

    sleep(2);

    $performance_data = $this->collectPerformanceData(function () {

      $this->drupalGet('node/add/article');

      sleep(2);

      $this->drupalGet('node/add/recipe');

      sleep(2);

      $this->drupalGet('node/add/page');

    }, 'umamiNodeAddEditor');

    $expected = [

      'ScriptCount' => 15,

      'ScriptBytes' => 3873548,

      'StylesheetCount' => 6,

      'StylesheetBytes' => 683604,

    ];

    $this->assertMetrics($expected, $performance_data);

  }