Improve CKEditorTestTrait

Copying over feedback from #2994699-52: Create a CKEditor plugin to select and embed a media item from the Media Library

1.

+++ b/core/modules/ckeditor/tests/src/Traits/CKEditorTestTrait.php
@@ -0,0 +1,103 @@
+   * @param string $instance_id
+   *   The CKEditor instance ID.

This parameter description needs to start with (optional) and state that it defaults to 'edit-body-0-value'.

2.

+++ b/core/modules/ckeditor/tests/src/Traits/CKEditorTestTrait.php
@@ -0,0 +1,103 @@
+    $this->getSession()->wait($timeout, $condition);

It might be preferable for us to use $this->assertSession()->assertJsCondition() here, since that will cause the test to fail (and rightfully so), if CKEditor is not ready in time.

3.

+++ b/core/modules/ckeditor/tests/src/Traits/CKEditorTestTrait.php
@@ -0,0 +1,103 @@
+  protected function assignNameToCkeditorIframe() {
+    $javascript = <<<JS
+(function(){
+  document.getElementsByClassName('cke_wysiwyg_frame')[0].id = 'ckeditor';
+})()
+JS;
+    $this->getSession()->evaluateScript($javascript);
+  }

What if there is more than one CKEditor instance on the page? Maybe this method should accept an $index parameter, defaulting to 0, to choose which CKEditor to work with. And then it can use that index to generate a unique name, like "ckeditor_0". Alternately, we could have this method loop through every CKEditor frame (getElementsByClassName() returns a set of elements anyway) and assign a unique one to each: "ckeditor_0", "ckeditor_1", etc.

4.

+++ b/core/modules/ckeditor/tests/src/Traits/CKEditorTestTrait.php
@@ -0,0 +1,103 @@
+  protected function pressEditorButton($name) {
+    $this->getSession()->switchToIFrame();
+    $button = $this->assertSession()->waitForElementVisible('css', 'a.cke_button__' . $name);
+    $this->assertNotEmpty($button);
+    $button->click();
+  }

Again, what if there are multiple CKEditor instances?

This doesn't need to be postponed anymore, now that #2994699: Create a CKEditor plugin to select and embed a media item from the Media Library has landed.

Cleanup.

Note: This was backported to 8.7 yesterday: #3076609: \Drupal\Tests\ckeditor\FunctionalJavascript\CKEditorIntegrationTest fails on Sqlite, so if this issue is resolved the changes should go into that branch too.

This looks good to me. Most of the changes were initially from #2994699-52: Create a CKEditor plugin to select and embed a media item from the Media Library, so this has been reviewed there too :)

Setting back to RTBC, it looks like the failure was unrelated. I reran testing 8.9 against patch #4 and it was all green.

Crediting @Wim Leers and @phenaproxima for review on the other issue.

Committed and pushed b28a392ece to 9.0.x and 5a4dd40a42 to 8.9.x and 1bce7388c9 to 8.8.x. Thanks!

Backported to 8.8.x as a test-only improvement.