From f6351459ac82f8fdf5fd3cb6a0e72d3da9f2e32c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ella=20Van=C2=A0Dorpe?= <iseulde@automattic.com>
Date: Fri, 16 Nov 2018 13:02:30 +0100
Subject: [PATCH] RichText: fix caret position when deleting a selected word
 (#11965)

* Add test case

* Try native delete behaviour for uncollpased selection

* Handle deletion if rich text is entirely selected

* Remove unnecessary code
---
 packages/dom/src/dom.js                       | 16 +++-
 .../editor/src/components/rich-text/index.js  | 10 +--
 .../src/components/rich-text/tinymce.js       |  9 +-
 .../__snapshots__/writing-flow.test.js.snap   | 82 +++++++++++++++++++
 test/e2e/specs/writing-flow.test.js           | 60 +++++++++-----
 5 files changed, 143 insertions(+), 34 deletions(-)

diff --git a/packages/dom/src/dom.js b/packages/dom/src/dom.js
index 9f65e47559ac33..c718cdbebe1907 100644
--- a/packages/dom/src/dom.js
+++ b/packages/dom/src/dom.js
@@ -497,11 +497,25 @@ export function isEntirelySelected( element ) {
 
 	const { startContainer, endContainer, startOffset, endOffset } = range;
 
-	return (
+	if (
 		startContainer === element &&
 		endContainer === element &&
 		startOffset === 0 &&
 		endOffset === element.childNodes.length
+	) {
+		return true;
+	}
+
+	const lastChild = element.lastChild;
+	const lastChildContentLength = lastChild.nodeType === TEXT_NODE ?
+		lastChild.data.length :
+		lastChild.childNodes.length;
+
+	return (
+		startContainer === element.firstChild &&
+		endContainer === element.lastChild &&
+		startOffset === 0 &&
+		endOffset === lastChildContentLength
 	);
 }
 
diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js
index 3bda199650d234..050421c57fdaa9 100644
--- a/packages/editor/src/components/rich-text/index.js
+++ b/packages/editor/src/components/rich-text/index.js
@@ -510,8 +510,8 @@ export class RichText extends Component {
 			const start = getSelectionStart( value );
 			const end = getSelectionEnd( value );
 
-			// Always handle uncollapsed selections ourselves.
-			if ( ! isCollapsed( value ) ) {
+			// Always handle full content deletion ourselves.
+			if ( start === 0 && end !== 0 && end === value.text.length ) {
 				this.onChange( remove( value ) );
 				event.preventDefault();
 				return;
@@ -600,12 +600,6 @@ export class RichText extends Component {
 	 *                            keyboard.
 	 */
 	onKeyUp( { keyCode } ) {
-		// The input event does not fire when the whole field is selected and
-		// BACKSPACE is pressed.
-		if ( keyCode === BACKSPACE ) {
-			this.onChange( this.createRecord() );
-		}
-
 		// `scrollToRect` is called on `nodechange`, whereas calling it on
 		// `keyup` *when* moving to a new RichText element results in incorrect
 		// scrolling. Though the following allows false positives, it results
diff --git a/packages/editor/src/components/rich-text/tinymce.js b/packages/editor/src/components/rich-text/tinymce.js
index 91dc9612c9ae13..3a2fa697ec8a0c 100644
--- a/packages/editor/src/components/rich-text/tinymce.js
+++ b/packages/editor/src/components/rich-text/tinymce.js
@@ -10,6 +10,7 @@ import classnames from 'classnames';
  */
 import { Component, createElement } from '@wordpress/element';
 import { BACKSPACE, DELETE, ENTER, LEFT, RIGHT } from '@wordpress/keycodes';
+import { isEntirelySelected } from '@wordpress/dom';
 
 /**
  * Internal dependencies
@@ -272,11 +273,13 @@ export default class TinyMCE extends Component {
 
 	onKeyDown( event ) {
 		const { keyCode } = event;
-		const { startContainer, startOffset, endContainer, endOffset } = getSelection().getRangeAt( 0 );
-		const isCollapsed = startContainer === endContainer && startOffset === endOffset;
+		const isDelete = keyCode === DELETE || keyCode === BACKSPACE;
 
 		// Disables TinyMCE behaviour.
-		if ( keyCode === ENTER || ( ! isCollapsed && ( keyCode === DELETE || keyCode === BACKSPACE ) ) ) {
+		if (
+			keyCode === ENTER ||
+			( isDelete && isEntirelySelected( this.editorNode ) )
+		) {
 			event.preventDefault();
 			// For some reason this is needed to also prevent the insertion of
 			// line breaks.
diff --git a/test/e2e/specs/__snapshots__/writing-flow.test.js.snap b/test/e2e/specs/__snapshots__/writing-flow.test.js.snap
index ee6bcfc536604e..77e8a9596b7729 100644
--- a/test/e2e/specs/__snapshots__/writing-flow.test.js.snap
+++ b/test/e2e/specs/__snapshots__/writing-flow.test.js.snap
@@ -32,6 +32,52 @@ exports[`adding blocks should clean TinyMCE content 2`] = `
 <!-- /wp:paragraph -->"
 `;
 
+exports[`adding blocks should create valid paragraph blocks when rapidly pressing Enter 1`] = `
+"<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->"
+`;
+
 exports[`adding blocks should insert line break at end 1`] = `
 "<!-- wp:paragraph -->
 <p>a<br><br></p>
@@ -75,3 +121,39 @@ exports[`adding blocks should navigate around inline boundaries 1`] = `
 <p>BeforeThird</p>
 <!-- /wp:paragraph -->"
 `;
+
+exports[`adding blocks should not delete surrounding space when deleting a selected word 1`] = `
+"<!-- wp:paragraph -->
+<p>alpha  gamma</p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`adding blocks should not delete surrounding space when deleting a selected word 2`] = `
+"<!-- wp:paragraph -->
+<p>alpha beta gamma</p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`adding blocks should not delete surrounding space when deleting a word with Alt+Backspace 1`] = `
+"<!-- wp:paragraph -->
+<p>alpha  gamma</p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`adding blocks should not delete surrounding space when deleting a word with Alt+Backspace 2`] = `
+"<!-- wp:paragraph -->
+<p>alpha beta gamma</p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`adding blocks should not delete surrounding space when deleting a word with Backspace 1`] = `
+"<!-- wp:paragraph -->
+<p>1  3</p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`adding blocks should not delete surrounding space when deleting a word with Backspace 2`] = `
+"<!-- wp:paragraph -->
+<p>1 2 3</p>
+<!-- /wp:paragraph -->"
+`;
diff --git a/test/e2e/specs/writing-flow.test.js b/test/e2e/specs/writing-flow.test.js
index 0da4e2f29ad8af..0da85a9ff4dbe7 100644
--- a/test/e2e/specs/writing-flow.test.js
+++ b/test/e2e/specs/writing-flow.test.js
@@ -224,42 +224,58 @@ describe( 'adding blocks', () => {
 		expect( isInBlock ).toBe( true );
 	} );
 
-	it( 'should not delete trailing spaces when deleting a word with backspace', async () => {
+	it( 'should not delete surrounding space when deleting a word with Backspace', async () => {
 		await clickBlockAppender();
-		await page.keyboard.type( '1 2 3 4' );
+		await page.keyboard.type( '1 2 3' );
+		await pressTimes( 'ArrowLeft', ' 3'.length );
 		await page.keyboard.press( 'Backspace' );
-		await page.keyboard.type( '4' );
-		const blockText = await page.evaluate( () => document.activeElement.textContent );
-		expect( blockText ).toBe( '1 2 3 4' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+
+		await page.keyboard.type( '2' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
 
-	it( 'should not delete trailing spaces when deleting a word with alt + backspace', async () => {
+	it( 'should not delete surrounding space when deleting a word with Alt+Backspace', async () => {
 		await clickBlockAppender();
-		await page.keyboard.type( 'alpha beta gamma delta' );
+		await page.keyboard.type( 'alpha beta gamma' );
+		await pressTimes( 'ArrowLeft', ' gamma'.length );
+
 		if ( process.platform === 'darwin' ) {
 			await pressWithModifier( 'Alt', 'Backspace' );
 		} else {
 			await pressWithModifier( META_KEY, 'Backspace' );
 		}
-		await page.keyboard.type( 'delta' );
-		const blockText = await page.evaluate( () => document.activeElement.textContent );
-		expect( blockText ).toBe( 'alpha beta gamma delta' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+
+		await page.keyboard.type( 'beta' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+	} );
+
+	it( 'should not delete surrounding space when deleting a selected word', async () => {
+		await clickBlockAppender();
+		await page.keyboard.type( 'alpha beta gamma' );
+		await pressTimes( 'ArrowLeft', ' gamma'.length );
+		await page.keyboard.down( 'Shift' );
+		await pressTimes( 'ArrowLeft', 'beta'.length );
+		await page.keyboard.up( 'Shift' );
+		await page.keyboard.press( 'Backspace' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+
+		await page.keyboard.type( 'beta' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
 
 	it( 'should create valid paragraph blocks when rapidly pressing Enter', async () => {
 		await clickBlockAppender();
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
-		await page.keyboard.press( 'Enter' );
+		await pressTimes( 'Enter', 10 );
+
 		// Check that none of the paragraph blocks have <br> in them.
-		const postContent = await getEditedPostContent();
-		expect( postContent.indexOf( 'br' ) ).toBe( -1 );
+		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
 } );