Writing Flow: Caret position not always preserved in vertical traversal

[aduth]

Describe the bug

When vertically navigating between paragraphs using ArrowUp, ArrowDown, the caret does not always land where would be expected to preserve its horizontal position.

It seems to be impacted by the field "remembering" where the caret was when last focused, rather than where it should be.

To reproduce

Steps to reproduce the behavior:

1. Navigate to Posts > Add New
2. Click the writing prompt
3. Add a paragraph of text "abc"
4. Press Enter
5. Add a paragraph of text "123"
6. Press ArrowUp
7. Press ArrowLeft 3 times
8. Press ArrowDown
9. Note that the caret is placed at the end of the second paragraph

Expected behavior

The caret should be placed at roughly equivalent horizontal offset as where it originated from in the first paragraph (i.e. at the start of the paragraph).

Screenshots

Desktop:

• OS: macOS 10.14.4 (18E226)
• Browser: Chrome Version 74.0.3729.131

[aduth]

The result of some initial debugging:

The underlying cause of the issue is that we set verticalRect only when unset, and only clear it when either clicking within the WritingFlow area, or when WritingFlow handles horizontal navigation.

gutenberg/packages/block-editor/src/components/writing-flow/index.js

Lines 284 to 288 in 449c254

	if ( ! isVertical ) {
	this.verticalRect = null;
	} else if ( ! this.verticalRect ) {
	this.verticalRect = computeCaretRect();
	}

gutenberg/packages/block-editor/src/components/writing-flow/index.js

Line 348 in 449c254

onMouseDown={ this.clearVerticalRect }

This has been the behavior since its original introduction in #2988.

I expect this regressed as an indirect result of #13697 (change), specifically that RichText calls to Event#preventDefault in its own handling of horizontal arrow navigation:

gutenberg/packages/block-editor/src/components/rich-text/index.js

Lines 769 to 770 in 449c254

	// In all other cases, prevent default behaviour.
	event.preventDefault();

Since WritingFlow respects defaultPrevented, the logic to unset verticalRect for horizontal navigation will not be reached.

gutenberg/packages/block-editor/src/components/writing-flow/index.js

Lines 272 to 276 in 449c254

	// Abort if navigation has already been handled (e.g. RichText inline
	// boundaries).
	if ( event.nativeEvent.defaultPrevented ) {
	return;
	}

When the next vertical traversal occurs then, it uses the old, inaccurate verticalRect value, since it was never unset in the interim.

gutenberg/packages/block-editor/src/components/writing-flow/index.js

Line 316 in 449c254

placeCaretAtVerticalEdge( closestTabbable, isReverse, this.verticalRect );

Proposed solution:

It's not clear to me that we even need verticalRect to be an instance variable, vs. computed on-demand at the logic above for placeCaretAtVerticalRect. Doing so appears to resolve the issue in some brief testing.

Based on the onMouseDown reset behavior, it may have been partly necessary for either multi-selection or non-collapsed selections? The mouse dependency is concerning nonetheless, and existing writing-flow.test.js end-to-end tests pass without the instance variable (which would indicate we need better tests if the instance variable is necessary).

cc @ellatrix

[aduth]

Fixed by #15624 (an alternative to #15607).