RichText: don't set selection during selectionchange

[ellatrix]

Description

Fixes #13895. When the browser updates the selection, we are currently updating the DOM and resetting selection (as we are moving towards RichText becoming a controlled component). Updating the DOM is needed to add the boundary class to formatting elements when the caret moves inside the element. Setting the selection is not really needed, but I thought it'd do no harm.

But... the browser may set the selection differently than we do. E.g. it may set the selection at the end of the text node before the formatting element, where we would set it at the start of the text element inside the formatting element. This causes isRangeEqual to fail (the range is not equal, but visually the same), and we will reset selection during mousedown and mouseup. Selection setting by the browser will be resumed at the selection we set.

The solution is to temporarily remove selection setting until we find a way to prevent it if the selection is at the same text index.

How has this been tested?

Screenshots

Types of changes

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[aduth]

Part of me wonders if this is the sort of thing where it'd be easier to just pass through whatever additional options verbatim, i.e. ...options from a second object argument. But I can also see the opposite argument of there being value in being explicit, especially in this case where it's something perhaps temporary.

[ellatrix]

Yeah, I definitely see this as something temporary.

[ellatrix]

I'm not sure if I can make a test case for this. :/

[aduth]

@iseulde The third GIF in #13895 is keyboard-only and appears like it's resolved by the changes here, in case that could work well as a base for an E2E test case.

[ellatrix]

Ah, it does, thanks!

[aduth]

With a basic understanding of the underlying issue and the purpose of the selection monitoring in applying the inline boundary highlighting, I think this is a sensible change that works well in my testing. Though I'd agree as well to your further point that we should be able to have more confidence that a selection we'd assign would match a browser behavior.

[ellatrix]

I tried adding the following test, but it passes in puppeteer before this change.

	it( 'should not lose selection direction', async () => {
		await clickBlockAppender();
		await pressKeyWithModifier( 'primary', 'b' );
		await page.keyboard.type( '1' );
		await pressKeyWithModifier( 'primary', 'b' );
		await page.keyboard.type( '23' );
		await page.keyboard.press( 'ArrowLeft' );
		await page.keyboard.down( 'Shift' );
		await page.keyboard.press( 'ArrowLeft' );
		await page.keyboard.press( 'ArrowLeft' );
		await page.keyboard.press( 'ArrowRight' );
		await page.keyboard.press( 'ArrowRight' );
		await page.keyboard.up( 'Shift' );

		// There should be no selection. The following should not do anything.
		await pressKeyWithModifier( 'primary', 'b' );

		expect( await getEditedPostContent() ).toMatchSnapshot();
	} );

[ellatrix]

By passing, I meant that it creates the following, as expected one would expect:

<!-- wp:paragraph -->
<p><strong>1</strong>23</p>
<!-- /wp:paragraph -->

[aduth]

For me, Cmd+B does nothing in both the case where the selection is correct (this branch) and when incorrect (on master). I'm not sure if that's a bug as well. Maybe the test should type a normal letter instead in the mean time (which would destroy content if the buggy selection behavior exists as on master, and insert a character as in this branch)?

[aduth]

Example:

it( 'should not lose selection direction', async () => {
	await clickBlockAppender();
	await pressKeyWithModifier( 'primary', 'b' );
	await page.keyboard.type( '1' );
	await pressKeyWithModifier( 'primary', 'b' );
	await page.keyboard.type( '24' );
	await page.keyboard.press( 'ArrowLeft' );
	await page.keyboard.down( 'Shift' );
	await page.keyboard.press( 'ArrowLeft' );
	await page.keyboard.press( 'ArrowLeft' );
	await page.keyboard.press( 'ArrowRight' );
	await page.keyboard.press( 'ArrowRight' );
	await page.keyboard.up( 'Shift' );

	// There should be no selection. The following should insert.
	await page.keyboard.type( '3' );

	expect( await getEditedPostContent() ).toMatchSnapshot();
} );

[ellatrix]

Added an e2e test and everything passes. Confirmed that it fails for master.