Issue #3479411 by phenaproxima, thejimbirch: The addItemToToolbar config...

Issue #3479411 by phenaproxima, thejimbirch: The addItemToToolbar config action should not, by default, add items that are already in the toolbar

core/modules/ckeditor5/src/Plugin/ConfigAction/AddItemToToolbar.php

@@ -47,30 +47,40 @@ public function apply(string $configName, mixed $value): void {
       throw new ConfigActionException(sprintf('The %s config action only works with editors that use CKEditor 5.', $this->pluginId));
     }
 
-    $editor_settings = $editor->getSettings();
     if (is_string($value)) {
-      $editor_settings['toolbar']['items'][] = $item_name = $value;
+      $value = ['item_name' => $value];
     }
-    else {
-      assert(is_array($value));
+    assert(is_array($value));
 
-      $item_name = $value['item_name'];
-      assert(is_string($item_name));
+    $item_name = $value['item_name'];
+    assert(is_string($item_name));
 
-      $replace = $value['replace'] ?? FALSE;
-      assert(is_bool($replace));
+    $replace = $value['replace'] ?? FALSE;
+    assert(is_bool($replace));
 
-      $position = $value['position'] ?? NULL;
-      if (is_int($position)) {
-        // If we want to replace the item at this position, then `replace`
-        // should be true. This would be useful if, for example, we wanted to
-        // replace the Image button with the Media Library.
-        array_splice($editor_settings['toolbar']['items'], $position, $replace ? 1 : 0, $item_name);
-      }
-      else {
-        $editor_settings['toolbar']['items'][] = $item_name;
-      }
+    $position = $value['position'] ?? NULL;
+
+    $allow_duplicate = $value['allow_duplicate'] ?? FALSE;
+    assert(is_bool($allow_duplicate));
+
+    $editor_settings = $editor->getSettings();
+
+    // If the item is already in the toolbar and we're not allowing duplicate
+    // items, we're done.
+    if (in_array($item_name, $editor_settings['toolbar']['items'], TRUE) && $allow_duplicate === FALSE && $item_name !== '|') {
+      return;
+    }
+
+    if (is_int($position)) {
+      // If we want to replace the item at this position, then `replace`
+      // should be true. This would be useful if, for example, we wanted to
+      // replace the Image button with the Media Library.
+      array_splice($editor_settings['toolbar']['items'], $position, $replace ? 1 : 0, $item_name);
     }
+    else {
+      $editor_settings['toolbar']['items'][] = $item_name;
+    }
+
     // If we're just adding a vertical separator, there's nothing else we need
     // to do at this point.
     if ($item_name === '|') {

core/modules/ckeditor5/tests/src/Kernel/ConfigAction/AddItemToToolbarConfigActionTest.php

@@ -69,6 +69,8 @@ protected function setUp(): void {
    *   [{"item_name": "sourceEditing"}, ["heading", "bold", "italic", "sourceEditing"]]
    *   [{"item_name": "sourceEditing", "position": 1}, ["heading", "sourceEditing", "bold", "italic"]]
    *   [{"item_name": "sourceEditing", "position": 1, "replace": true}, ["heading", "sourceEditing", "italic"]]
+   *   [{"item_name": "bold"}, ["heading", "bold", "italic"]]
+   *   [{"item_name": "bold", "allow_duplicate": true}, ["heading", "bold", "italic", "bold"]]
    */
   public function testAddItemToToolbar(string|array $action, array $expected_toolbar_items): void {
     $recipe = $this->createRecipe([
@@ -86,8 +88,11 @@ public function testAddItemToToolbar(string|array $action, array $expected_toolb
     /** @var array{toolbar: array{items: string[]}, plugins: array<string, array<mixed>>} $settings */
     $settings = Editor::load('filter_test')?->getSettings();
     $this->assertSame($expected_toolbar_items, $settings['toolbar']['items']);
+
     // The plugin's default settings should have been added.
-    $this->assertSame([], $settings['plugins']['ckeditor5_sourceEditing']['allowed_tags']);
+    if (in_array('sourceEditing', $expected_toolbar_items, TRUE)) {
+      $this->assertSame([], $settings['plugins']['ckeditor5_sourceEditing']['allowed_tags']);
+    }
   }
 
   public function testAddNonExistentItem(): void {