From 3518cddca7cf520cf4cac6b941695b7c37543758 Mon Sep 17 00:00:00 2001
From: Dennis Snell <dennis.snell@automattic.com>
Date: Sun, 10 Mar 2019 16:49:11 -0700
Subject: [PATCH] Fix: Pasting captions without an image fails

Fixes #13527
See original work in #12315

When pasting content which includes the `[caption]` shortcode we assume
that the content is well-formed (that there is not only an `<img />` in
there but also in the first position).

In this patch we fix the problem by removing the old code, which removed
the first `Element` node, and replaced it with code that removes the
first `IMG` node _if one is found_.

We're leaving other image nodes in place in case the caption contains
image nodes and we're not requiring that the `IMG` be the first child
of the caption shortcode in case people are wrapping the image in other
valid HTML like this...

```html
[caption]<a href="some.link"><img src="some.image"></a>[/caption]
```

See the new unit tests for a more complete specification of the
intended behaviors.

PR reviewed, debugged, and created by:
-> LANNISTER MOB <-
 - @codebykat
 - @dmsnell
 - @gwwar
 - @kwight
 - @mmtr
 - @obenland
 - @rodrigoi
 - @vindl
---
 packages/block-library/src/image/index.js     | 22 +++++++++------
 .../block-library/src/image/test/index.js     | 28 +++++++++++++++++++
 2 files changed, 42 insertions(+), 8 deletions(-)
 create mode 100644 packages/block-library/src/image/test/index.js

diff --git a/packages/block-library/src/image/index.js b/packages/block-library/src/image/index.js
index 92b9af7ec9a42c..ba0f9d1b237ac1 100644
--- a/packages/block-library/src/image/index.js
+++ b/packages/block-library/src/image/index.js
@@ -123,6 +123,19 @@ function getFirstAnchorAttributeFormHTML( html, attributeName ) {
 	}
 }
 
+export function stripFirstImage( attributes, { shortcode } ) {
+	const { body } = document.implementation.createHTMLDocument( '' );
+
+	body.innerHTML = shortcode.content;
+
+	const firstImage = body.querySelector( 'img' );
+	if ( firstImage ) {
+		firstImage.parentNode.removeChild( firstImage );
+	}
+
+	return body.innerHTML.trim();
+}
+
 export const settings = {
 	title: __( 'Image' ),
 
@@ -196,14 +209,7 @@ export const settings = {
 						selector: 'img',
 					},
 					caption: {
-						shortcode: ( attributes, { shortcode } ) => {
-							const { body } = document.implementation.createHTMLDocument( '' );
-
-							body.innerHTML = shortcode.content;
-							body.removeChild( body.firstElementChild );
-
-							return body.innerHTML.trim();
-						},
+						shortcode: stripFirstImage,
 					},
 					href: {
 						shortcode: ( attributes, { shortcode } ) => {
diff --git a/packages/block-library/src/image/test/index.js b/packages/block-library/src/image/test/index.js
new file mode 100644
index 00000000000000..4cd5a8c5b9cbbb
--- /dev/null
+++ b/packages/block-library/src/image/test/index.js
@@ -0,0 +1,28 @@
+/**
+ * Internal dependencies
+ */
+import { stripFirstImage } from '../';
+
+describe( 'stripFirstImage', () => {
+	test( 'should do nothing if no image is present', () => {
+		expect( stripFirstImage( {}, { shortcode: { content: '' } } ) ).toEqual( '' );
+		expect( stripFirstImage( {}, { shortcode: { content: 'Tucson' } } ) ).toEqual( 'Tucson' );
+		expect( stripFirstImage( {}, { shortcode: { content: '<em>Tucson</em>' } } ) ).toEqual( '<em>Tucson</em>' );
+	} );
+
+	test( 'should strip out image when leading as expected', () => {
+		expect( stripFirstImage( {}, { shortcode: { content: '<img>' } } ) ).toEqual( '' );
+		expect( stripFirstImage( {}, { shortcode: { content: '<img>Image!' } } ) ).toEqual( 'Image!' );
+		expect( stripFirstImage( {}, { shortcode: { content: '<img src="image.png">Image!' } } ) ).toEqual( 'Image!' );
+	} );
+
+	test( 'should strip out image when not in leading position as expected', () => {
+		expect( stripFirstImage( {}, { shortcode: { content: 'Before<img>' } } ) ).toEqual( 'Before' );
+		expect( stripFirstImage( {}, { shortcode: { content: 'Before<img>Image!' } } ) ).toEqual( 'BeforeImage!' );
+		expect( stripFirstImage( {}, { shortcode: { content: 'Before<img src="image.png">Image!' } } ) ).toEqual( 'BeforeImage!' );
+	} );
+
+	test( 'should strip out only the first of many images', () => {
+		expect( stripFirstImage( {}, { shortcode: { content: '<img><img>' } } ) ).toEqual( '<img>' );
+	} );
+} );