Rich Text: Detect handled horizontal navigation by preventDefault (#7644

)

editor/components/rich-text/index.js

@@ -212,21 +212,6 @@ export class RichText extends Component {
 		this.editor.shortcuts.add( rawShortcut.primary( 'z' ), '', 'Undo' );
 		this.editor.shortcuts.add( rawShortcut.primaryShift( 'z' ), '', 'Redo' );
 
-		// Bind directly to the document, since properties assigned to TinyMCE
-		// events are lost when accessed during bubbled handlers. This must be
-		// captured _before_ TinyMCE's internal handling of arrow keys, which
-		// would otherwise already have moved the caret position. This is why
-		// it is bound to the capture phase.
-		//
-		// Note: This is ideally a temporary measure, only needed so long as
-		// TinyMCE inaccurately prevents default around inline boundaries.
-		// Ideally we rely on the defaultPrevented property to stop WritingFlow
-		// from transitioning.
-		//
-		// See: https://github.com/tinymce/tinymce/issues/4476
-		// See: WritingFlow#onKeyDown
-		this.editor.dom.doc.addEventListener( 'keydown', this.onHorizontalNavigationKeyDown, true );
-
 		// Remove TinyMCE Core shortcut for consistency with global editor
 		// shortcuts. Also clashes with Mac browsers.
 		this.editor.shortcuts.remove( 'meta+y', '', 'Redo' );
@@ -467,53 +452,46 @@ export class RichText extends Component {
 	}
 
 	/**
-	 * Handles a horizontal navigation key down event to stop propagation if it
-	 * can be inferred that it will be handled by TinyMCE (notably transitions
-	 * out of an inline boundary node).
+	 * Handles a horizontal navigation key down event to handle the case where
+	 * TinyMCE attempts to preventDefault when on the outside edge of an inline
+	 * boundary when arrowing _away_ from the boundary, not within it. Replaces
+	 * the TinyMCE event `preventDefault` behavior with a noop, such that those
+	 * relying on `defaultPrevented` are not misinformed about the arrow event.
+	 *
+	 * If TinyMCE#4476 is resolved, this handling may be removed.
+	 *
+	 * @see https://github.com/tinymce/tinymce/issues/4476
 	 *
-	 * @param {KeyboardEvent} event Keydown event.
+	 * @param {tinymce.EditorEvent<KeyboardEvent>} event Keydown event.
 	 */
 	onHorizontalNavigationKeyDown( event ) {
-		const { keyCode } = event;
-		const isHorizontalNavigation = keyCode === LEFT || keyCode === RIGHT;
-		if ( ! isHorizontalNavigation ) {
-			return;
-		}
-
-		const { focusNode, focusOffset } = window.getSelection();
+		const { focusNode } = window.getSelection();
 		const { nodeType, nodeValue } = focusNode;
 
 		if ( nodeType !== Node.TEXT_NODE ) {
 			return;
 		}
 
-		const isReverse = event.keyCode === LEFT;
-
-		// Look to previous character for ZWSP if navigating in reverse.
-		let offset = focusOffset;
-		if ( isReverse ) {
-			offset--;
+		if ( nodeValue.length !== 1 || nodeValue[ 0 ] !== TINYMCE_ZWSP ) {
+			return;
 		}
 
-		// Workaround: In a new inline boundary node, the zero-width space
-		// wrongly lingers at the beginning of the node, rather than following
-		// the caret. If we are at the extent of the inline boundary, but the
-		// ZWSP exists at the beginning, consider as though it were to be
-		// handled as a transition outside the inline boundary.
-		//
-		// Since this condition could also be satisfied in the case that we're
-		// on the right edge of an inline boundary -- where the ZWSP exists as
-		// as an otherwise empty focus node -- ensure that the focus node is
-		// non-empty.
-		//
-		// See: https://github.com/tinymce/tinymce/issues/4472
-		if ( ! isReverse && focusOffset === nodeValue.length &&
-				nodeValue.length > 1 && nodeValue[ 0 ] === TINYMCE_ZWSP ) {
-			offset = 0;
-		}
+		const { keyCode } = event;
 
-		if ( nodeValue[ offset ] === TINYMCE_ZWSP ) {
-			event._navigationHandled = true;
+		// Consider to be moving away from inline boundary based on:
+		//
+		// 1. Within a text fragment consisting only of ZWSP.
+		// 2. If in reverse, there is no previous sibling. If forward, there is
+		//    no next sibling (i.e. end of node).
+		const isReverse = keyCode === LEFT;
+		const edgeSibling = isReverse ? 'previousSibling' : 'nextSibling';
+		if ( ! focusNode[ edgeSibling ] ) {
+			// Note: This is not reassigning on the native event, rather the
+			// "fixed" TinyMCE copy, which proxies its preventDefault to the
+			// native event. By reassigning here, we're effectively preventing
+			// the proxied call on the native event, but not otherwise mutating
+			// the original event object.
+			event.preventDefault = noop;
 		}
 	}
 
@@ -555,6 +533,11 @@ export class RichText extends Component {
 			event.stopImmediatePropagation();
 		}
 
+		const isHorizontalNavigation = keyCode === LEFT || keyCode === RIGHT;
+		if ( isHorizontalNavigation ) {
+			this.onHorizontalNavigationKeyDown( event );
+		}
+
 		// If we click shift+Enter on inline RichTexts, we avoid creating two contenteditables
 		// We also split the content and call the onSplit prop if provided.
 		if ( keyCode === ENTER ) {

editor/components/writing-flow/index.js

@@ -188,13 +188,9 @@ class WritingFlow extends Component {
 	onKeyDown( event ) {
 		const { hasMultiSelection, onMultiSelect, blocks } = this.props;
 
-		// If navigation has already been handled (e.g. TinyMCE inline
-		// boundaries), abort. Ideally this uses Event#defaultPrevented. This
-		// is currently not possible because TinyMCE will misreport an event
-		// default as prevented on outside edges of inline boundaries.
-		//
-		// See: https://github.com/tinymce/tinymce/issues/4476
-		if ( event.nativeEvent._navigationHandled ) {
+		// Aobrt if navigation has already been handled (e.g. TinyMCE inline
+		// boundaries).
+		if ( event.nativeEvent.defaultPrevented ) {
 			return;
 		}
 

test/e2e/specs/writing-flow.test.js

@@ -82,7 +82,20 @@ describe( 'adding blocks', () => {
 		await page.keyboard.up( 'Shift' );
 		await pressWithModifier( 'mod', 'b' );
 
-		// Arrow left from selected bold should traverse into first.
+		// Arrow left from selected bold should collapse to before the inline
+		// boundary. Arrow once more to traverse into first paragraph.
+		//
+		// See native behavior example: http://fiddle.tinymce.com/kvgaab
+		//
+		//  1. Select all of second paragraph, end to beginning
+		//  2. Press ArrowLeft
+		//  3. Type
+		//  4. Note that text is not bolded
+		//
+		// This is technically different than how other word processors treat
+		// the collapse while a bolded segment is selected, but our behavior
+		// is consistent with TinyMCE.
+		await page.keyboard.press( 'ArrowLeft' );
 		await page.keyboard.press( 'ArrowLeft' );
 		await page.keyboard.type( 'After' );