Escape Editable HTML (#17994)

* Escape HTML: Always Escape Ampersand

* Revert "Escape HTML: Always Escape Ampersand"

This reverts commit 70dfa53.

* Escape HTML for Editable Text

* Revert PlainText changes

* Adjust code block escape util

* Update package lock

* Add test case for escapeEditableHTML

* Update docs

packages/block-library/package.json

@@ -37,6 +37,7 @@
 		"@wordpress/deprecated": "file:../deprecated",
 		"@wordpress/editor": "file:../editor",
 		"@wordpress/element": "file:../element",
+		"@wordpress/escape-html": "file:../escape-html",
 		"@wordpress/i18n": "file:../i18n",
 		"@wordpress/is-shallow-equal": "file:../is-shallow-equal",
 		"@wordpress/keycodes": "file:../keycodes",

packages/block-library/src/code/edit.js

@@ -7,14 +7,13 @@ import { __ } from '@wordpress/i18n';
  * Internal dependencies
  */
 import { PlainText } from '@wordpress/block-editor';
-import { escape, unescape } from './utils';
 
 export default function CodeEdit( { attributes, setAttributes, className } ) {
 	return (
 		<div className={ className }>
 			<PlainText
-				value={ unescape( attributes.content ) }
-				onChange={ ( content ) => setAttributes( { content: escape( content ) } ) }
+				value={ attributes.content }
+				onChange={ ( content ) => setAttributes( { content } ) }
 				placeholder={ __( 'Write code…' ) }
 				aria-label={ __( 'Code' ) }
 			/>

packages/block-library/src/code/save.js

@@ -1,3 +1,8 @@
+/**
+ * Internal dependencies
+ */
+import { escape } from './utils';
+
 export default function save( { attributes } ) {
-	return <pre><code>{ attributes.content }</code></pre>;
+	return <pre><code>{ escape( attributes.content ) }</code></pre>;
 }

packages/block-library/src/code/test/utils.js

@@ -1,15 +1,10 @@
 /**
  * Internal dependencies
  */
-import { escape, unescape } from '../utils';
+import { escape } from '../utils';
 
 describe( 'core/code', () => {
 	describe( 'escape()', () => {
-		it( 'should escape ampersands', () => {
-			const text = escape( '&' );
-			expect( text ).toBe( '&' );
-		} );
-
 		it( 'should escape opening square brackets', () => {
 			const text = escape( '[shortcode][/shortcode]' );
 			expect( text ).toBe( '[shortcode][/shortcode]' );
@@ -24,39 +19,5 @@ describe( 'core/code', () => {
 			const text = escape( 'Text https://example.com/test/' );
 			expect( text ).toBe( 'Text https://example.com/test/' );
 		} );
-
-		it( 'should escape ampersands last', () => {
-			const text = escape( '[shortcode][/shortcode]' );
-			expect( text ).toBe( '[shortcode][/shortcode]' );
-			expect( text ).not.toBe( '[shortcode][/shortcode]' );
-		} );
-	} );
-
-	describe( 'unescape()', () => {
-		it( 'should unescape escaped ampersands', () => {
-			const text = unescape( '&' );
-			expect( text ).toBe( '&' );
-		} );
-
-		it( 'should unescape escaped opening square brackets', () => {
-			const text = unescape( '[shortcode][/shortcode]' );
-			expect( text ).toBe( '[shortcode][/shortcode]' );
-		} );
-
-		it( 'should unescape the escaped protocol of an isolated url', () => {
-			const text = unescape( 'https://example.com/test/' );
-			expect( text ).toBe( 'https://example.com/test/' );
-		} );
-
-		it( 'should revert the result of escape()', () => {
-			const ampersand = unescape( escape( '&' ) );
-			expect( ampersand ).toBe( '&' );
-
-			const squareBracket = unescape( escape( '[shortcode][/shortcode]' ) );
-			expect( squareBracket ).toBe( '[shortcode][/shortcode]' );
-
-			const url = unescape( escape( 'https://example.com/test/' ) );
-			expect( url ).toBe( 'https://example.com/test/' );
-		} );
 	} );
 } );

packages/block-library/src/code/utils.js

@@ -3,6 +3,11 @@
  */
 import { flow } from 'lodash';
 
+/**
+ * WordPress dependencies
+ */
+import { escapeEditableHTML } from '@wordpress/escape-html';
+
 /**
  * Escapes ampersands, shortcodes, and links.
  *
@@ -11,49 +16,12 @@ import { flow } from 'lodash';
  */
 export function escape( content ) {
 	return flow(
-		escapeAmpersands,
+		escapeEditableHTML,
 		escapeOpeningSquareBrackets,
 		escapeProtocolInIsolatedUrls
 	)( content || '' );
 }
 
-/**
- * Unescapes escaped ampersands, shortcodes, and links.
- *
- * @param {string} content Content with (maybe) escaped ampersands, shortcodes, and links.
- * @return {string} The given content with escaped characters unescaped.
- */
-export function unescape( content ) {
-	return flow(
-		unescapeProtocolInIsolatedUrls,
-		unescapeOpeningSquareBrackets,
-		unescapeAmpersands
-	)( content || '' );
-}
-
-/**
- * Returns the given content with all its ampersand characters converted
- * into their HTML entity counterpart (i.e. & => &)
- *
- * @param {string}  content The content of a code block.
- * @return {string} The given content with its ampersands converted into
- *                  their HTML entity counterpart (i.e. & => &)
- */
-function escapeAmpersands( content ) {
-	return content.replace( /&/g, '&' );
-}
-
-/**
- * Returns the given content with all & HTML entities converted into &.
- *
- * @param {string}  content The content of a code block.
- * @return {string} The given content with all & HTML entities
- *                  converted into &.
- */
-function unescapeAmpersands( content ) {
-	return content.replace( /&/g, '&' );
-}
-
 /**
  * Returns the given content with all opening shortcode characters converted
  * into their HTML entity counterpart (i.e. [ => [). For instance, a
@@ -71,16 +39,6 @@ function escapeOpeningSquareBrackets( content ) {
 	return content.replace( /\[/g, '[' );
 }
 
-/**
- * Returns the given content translating all [ into [.
- *
- * @param {string}  content The content of a code block.
- * @return {string} The given content with all [ into [.
- */
-function unescapeOpeningSquareBrackets( content ) {
-	return content.replace( /[/g, '[' );
-}
-
 /**
  * Converts the first two forward slashes of any isolated URL into their HTML
  * counterparts (i.e. // => //). For instance, https://youtube.com/watch?x
@@ -98,20 +56,3 @@ function unescapeOpeningSquareBrackets( content ) {
 function escapeProtocolInIsolatedUrls( content ) {
 	return content.replace( /^(\s*https?:)\/\/([^\s<>"]+\s*)$/m, '$1&#47;&#47;$2' );
 }
-
-/**
- * Converts the first two forward slashes of any isolated URL from the HTML entity
- * &#73; into /.
- *
- * An isolated URL is a URL that sits in its own line, surrounded only by spacing
- * characters.
- *
- * See https://github.com/WordPress/wordpress-develop/blob/5.1.1/src/wp-includes/class-wp-embed.php#L403
- *
- * @param {string}  content The content of a code block.
- * @return {string} The given content with the first two forward slashes of any
- *                  isolated URL from the HTML entity &#73; into /.
- */
-function unescapeProtocolInIsolatedUrls( content ) {
-	return content.replace( /^(\s*https?:)&#47;&#47;([^\s<>"]+\s*)$/m, '$1//$2' );
-}

packages/escape-html/README.md

@@ -64,6 +64,20 @@ _Returns_
 
 -   `string`: Escaped attribute value.
 
+<a name="escapeEditableHTML" href="#escapeEditableHTML">#</a> **escapeEditableHTML**
+
+Returns an escaped Editable HTML element value. This is different from
+`escapeHTML`, because for editable HTML, ALL ampersands must be escaped in
+order to render the content correctly on the page.
+
+_Parameters_
+
+-   _value_ `string`: Element value.
+
+_Returns_
+
+-   `string`: Escaped HTML element value.
+
 <a name="escapeHTML" href="#escapeHTML">#</a> **escapeHTML**
 
 Returns an escaped HTML element value.

packages/escape-html/src/index.js

@@ -96,6 +96,19 @@ export function escapeHTML( value ) {
 	return escapeLessThan( escapeAmpersand( value ) );
 }
 
+/**
+ * Returns an escaped Editable HTML element value. This is different from
+ * `escapeHTML`, because for editable HTML, ALL ampersands must be escaped in
+ * order to render the content correctly on the page.
+ *
+ * @param {string} value Element value.
+ *
+ * @return {string} Escaped HTML element value.
+ */
+export function escapeEditableHTML( value ) {
+	return escapeLessThan( value.replace( /&/g, '&' ) );
+}
+
 /**
  * Returns true if the given attribute name is valid, or false otherwise.
  *

packages/escape-html/src/test/index.js

@@ -8,6 +8,7 @@ import {
 	escapeAttribute,
 	escapeHTML,
 	isValidAttributeName,
+	escapeEditableHTML,
 } from '../';
 import __unstableEscapeGreaterThan from '../escape-greater';
 
@@ -94,3 +95,11 @@ describe( 'isValidAttributeName', () => {
 		expect( result ).toBe( true );
 	} );
 } );
+
+describe( 'escapeEditableHTML', () => {
+	it( 'should escape < and all ampersands', () => {
+		const result = escapeEditableHTML( '<a href="https://w.org">WP</a> & &lt;strong&gt;' );
+
+		expect( result ).toBe( '&lt;a href="https://w.org">WP&lt;/a> &amp; &amp;lt;strong&amp;gt;' );
+	} );
+} );

packages/rich-text/src/to-html-string.js

@@ -3,7 +3,7 @@
  */
 
 import {
-	escapeHTML,
+	escapeEditableHTML,
 	escapeAttribute,
 	isValidAttributeName,
 } from '@wordpress/escape-html';
@@ -106,6 +106,6 @@ function createElementHTML( { type, attributes, object, children } ) {
 
 function createChildrenHTML( children = [] ) {
 	return children.map( ( child ) => {
-		return child.text === undefined ? createElementHTML( child ) : escapeHTML( child.text );
+		return child.text === undefined ? createElementHTML( child ) : escapeEditableHTML( child.text );
 	} ).join( '' );
 }