Write test for CKEditor integration

Similar to #2560787-3: Add tests for entity access.

#2328659: Convert all existing entity_embed tests from WebTestBase to BrowserTestBase landed, unblocking this!

Here's an initial test.

Removing test code "createScreenshot".

Note: I had to add some extra code because of a bug in #2752253: Document correct filter_html_image_secure + filter_align + filter_caption + entity_embed filters order, write tests, and ensure correct by default. Once that issue is resolved, I can update the test.

Coding standard fix (makes comment multiline).

- Needs to cover "Ensure that the correct tag/attributes are added to the allowed HTML configuration when someone adds the button"

Adding check that allowed formats are automatically updated in the filter format.

In testing ImageFieldFormatter, I noticed that it requires "alt" and "title" attributes on the drupal-entity in order to function properly.

See: #3022768: Validate that `alt` and `title` are required attributes for `<drupal-entity>`, and ensure they're added by default.

I added a fix and a test class that adds functional test coverage. I would say that it's more test coverage for CKEditor integration, so it would be good to add that too.

I also noticed that there's some javascript testing that I missed in #13. When the user drags and drops the button into the toolbar it automatically updates the allowed_html via javascript. I didn't add test coverage for that, but rather after the form was saved. It would be easy to add.

So I'll mark this back as Needs Work.

- adds assertion before drop and drag that allowed_html doesn't have drupal-entity, and after drop and drag it does.

+    $allowed_html = $this->assertSession()->fieldExists('filters[filter_html][settings][allowed_html]')->getValue();
+    $this->assertNotContains('drupal-entity', $allowed_html);
+
+    $target = $this->assertSession()->waitForElementVisible('css', 'ul.ckeditor-toolbar-group-buttons');
+    $nodeButton = $this->assertSession()->elementExists('xpath', '//li[@data-drupal-ckeditor-button-name="node"]');
+    $nodeButton->dragTo($target);
+
+    $allowed_html_updated = $this->assertSession()->fieldExists('filters[filter_html][settings][allowed_html]')->getValue();
+    $this->assertContains('drupal-entity data-entity-type data-entity-uuid data-entity-embed-display data-entity-embed-display-settings data-align data-caption data-embed-button', $allowed_html_updated);

Hmm, not sure why that failed. It passed on my local drupalci_testbot. Here's a slightly reorganized version.

The order the attributes just needed to be reconfigured.

Changing $button to $this->button, which allows referencing it later. This will be useful because I plan to copy this test to test other usages.

O M G

EPIC progress here, thanks so much @oknate! 🥳🥳🙏

#9 has been fixed in that other issue, so this patch can become slightly simpler again :)

Fix nits.

+++ b/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
@@ -0,0 +1,170 @@
+    // Verify that the Entity Embed button shows up and results in an
+    // operational entity embedding experience in the text editor.
+    $this->drupalGet('/node/add/page');
+    $this->assertSession()->elementExists('css', 'a.cke_button__' . $this->button->id())->click();
+    $this->assertSession()->waitForId('drupal-modal');
+    $title = $this->node->label() . ' (' . $this->node->id() . ')';
+    $this->assertSession()->fieldExists('entity_id')->setValue($title);
+    $this->assertSession()->elementExists('css', 'button.js-button-next')->click();
+    $this->assertSession()->assertWaitOnAjaxRequest();
+    $this->assertSession()->responseContains('Selected entity');
+    $this->assertSession()->responseContains($this->node->label());
+    $this->assertSession()->elementExists('css', 'button.button--primary')->press();
+    $this->assertSession()->assertWaitOnAjaxRequest();
+    $this->assertSession()->waitForElementVisible('css', 'drupal-entity[data-embed-button="' . $this->button->id() . '"]');
+    $this->getSession()->getPage()->find('css', 'input[name="title[0][value]"]')->setValue('Testing Page with Embed');
+    $this->assertSession()->buttonExists('Save')->press();
+    $this->assertSession()->responseContains($this->node->label());

This is testing that a "Entity Embed" button in the text editor can insert an embedded entity into the text editor. (And that it is rendered when viewed.)

What this is not yet testing is that it's possible to edit an already existing <drupal-entity> in the content.

So adding something like

$this->drupalGet('/node/1/edit');
// double-click existing embed
// change something in dialog
// save dialog
// verify preview updated
// verify filtered result updated

would finish this.

Would you like to take his on? :)

#25 is just to to have complete functional testing, but don't get me wrong: I'm super grateful for how far you've already pushed this! 🙏🙏🙏 Excellent work, extremely valuable contributions! 🤩

Sure I can work on this. Thanks for the review!

LOVELY! Are you going to any Drupal events in the coming months? I totally owe you Belgian beer and chocolates for this :D

I don't have immediately plans. I'll let you know. Belgian beer and chocolates sounds great! If you're ever in New York, you can come visit me in the World Trade Center. It's a nice view.

Oh wow! My wife and I may be coming to visit NYC this summer, then we'll bring Belgian beer & chocolates and take you up on your offer! :D

That would be lovely, DM me when you know your plans.

Here's an updated patch that replaces the embed, per #25.

GREAT work, thanks so much @oknate!

1. +++ b/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
   @@ -90,20 +95,30 @@ class CKEditorIntegrationTest extends WebDriverTestBase {
   +      'title' => 'Billy Bones',
   +      'body' => [
   +        'value' => 'He lacks two fingers.',
   ...
   +      'title' => 'Long John Silver',
   +      'body' => [
   +        'value' => 'A one-legged seafaring man.',
   

   😆

2. +++ b/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
   @@ -90,20 +95,30 @@ class CKEditorIntegrationTest extends WebDriverTestBase {
   +        'format' => 'custom_format',
   

   Nit: this format does not exist. Fixed.

3. +++ b/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
   @@ -153,18 +170,58 @@ class CKEditorIntegrationTest extends WebDriverTestBase {
   +    // Go back to main iFrame.
   +    $this->getSession()->switchToIFrame();
   ...
   +    // Go back to main iFrame.
   +    $this->getSession()->switchToIFrame();
   

   There's no need for switching back if we didn't switch to something else first.

4. +++ b/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
   @@ -153,18 +170,58 @@ class CKEditorIntegrationTest extends WebDriverTestBase {
   +    // Test opening the dialog and switching embedded nodes.
   +    $this->assertSession()->waitForText('Billy Bones');
   

   This isn't quite testing what it seems to be testing; this was just waiting for the max timeout.

   That's because this is not switching to the CKEditor iframe, and it's only in that iframe that this text is supposed to appear.

   Fixed.

This addresses #32.3, adds a few additional comments to clarify what's being tested and adds matching before/after assertions to make the existing assertions more trustworthy and hence more valuable.

Still green — YAY — let's do this! This will certainly help ensure that #2544020: CKEditor plugin not written with CKEditor >=4.4 in mind; rewrite it based on core's `drupalimage` plugin does not introduce regressions! 🥳

Updating IS, assigning appropriate attribution.

UGH I just noticed that this was attributed to me instead of @oknate because I uploaded the last patch. d.o's default attribution algorithm often trips me up that way :( Very sorry, @oknate :/ Because you definitely are the primary author of this patch!

To compensate, I'll attribute you instead of myself on the next thing I actually am the primary author of.

No problem, thanks for getting it in! I would like to add test coverage for media to this test, since that's what we're looking to move into core. I'll look into that. Maybe a separate issue: Add test coverage for media embed.

If you're willing to do that, that'd be lovely! 🙏😃

That would also help #2998005: [PP-1] Support Drupal core's Media Library go faster 💪