Allow any HTML as content for replaceNodeContent

First, let's show that this patch's new test coverage indeed fails without the other changes.

I didn't change anything, I only rerolled it.

This is basically exactly the same problem as #2567561: Captioned elements and their children are removed when theme debugging on. We should use the same solution as Drupal core uses.

(The current code assumes only one DOM node will be present:

      // Load the contents into a new DOMDocument and retrieve the element.
      $replacement_node = Html::load($content)->getElementsByTagName('body')
        ->item(0)
        ->childNodes
        ->item(0);

This is identical to FilterCaption's buggy logic and what #2567561: Captioned elements and their children are removed when theme debugging on is fixing.

A duplicate of this bug was reported against the Entity Embed module: #2541984: <drupal-entity>is not replaced when twig debugging is enabled. It's a duplicate, because Entity Embed uses Embed's DomHelperTrait.

This is now blocking #2547085: Remove the <drupal-entity>/<div> placeholder instead of embedding the entity as a child, which is blocking #2628358: Disallow <div>, only allow <drupal-entity>, which again blocks its parent issue.

But… HAH! That won't actually work, because embed's replaceNodeContent() is broken: see #2602316: Allow any HTML as content for replaceNodeContent. It will only actually import the first DOM node. And the first DOM node when rendering an article template happens to be… a text DOM node with a newline in it. The second DOM node is the <article class="type-node" …> HTML that we expect.

So, with the above, tests actually fail for both patches because embed is broken…

Now working on #2602316: Allow any HTML as content for replaceNodeContent.

Hence, marking this critical, since it blocks so much.

+++ b/src/DomHelperTrait.php
@@ -85,25 +85,35 @@ trait DomHelperTrait {
+   * @return array
+   *   Array of DOMNode objects that replaced a node.
...
+    return $replacement_nodes_array;

This is an unnecessary API change.

So, instead of changing an API, let's just use the same solution that will land in Drupal 8 core: the one at #2567561: Captioned elements and their children are removed when theme debugging on.

Tests still needed!

Transplanted the tests written by @AndreyMaximov: modified them so they don't expect that API change anymore. In fact, having done that, I'm 90% certain he introduced that API change to simplify testability.

Fortunately, once you start using a data provider, that's not really necessary anymore :)

Oops. I unchecked the Display box for the non-FAIL patch. Hence it wasn't tested. Re-uploading that one.

There we go. Green :)

@WimLeers Should setNodeContent() also be fixed in the same way?

D'oh, no of course setNodeContent() doesn't need the same fix.

setNodeContent() apparently can handle string that consist of multiple nodes. Uploading updated test that proves that.

I will go ahead and fix this as there are patches that are waiting for this. Thanks!