RichText: only replace range and nodes if different

[ellatrix]

Description

When the selection that needs to be set is the same as the live selection, we shouldn't remove all ranges and add it again. When nodes that need to be set are equal, we also shouldn't replace them.

Decreases work done on a key press by about 10%.

Eliminates 2 extra selection change events some browsers emit even though they are emitted after a series of changes (so the selection stays the same during those emissions).

Eliminates extra layout and style recalculation.

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.

[ellatrix]

I updated this PR to include equal node checks. When typing, the live DOM is already updated, so we shouldn't needlessly replace element nodes if they remain equal. This prevents extra layout and style recalculations.

Now the work decrease is 10%, or more than 20% after merging #12510 into master.

[aduth]

Is future used after this function is complete? I'm wondering if there's any gains to be had by avoiding the removeChild operation and just stepping through the children.

let i;
for ( i = 0; i < future.childNodes.length; i++ ) {
	const futureChild = future.childNodes[ i ];
	const currentChild = current.childNodes[ i ];

	if ( ! currentChild ) {
		current.appendChild( futureChild );
	} else if ( ! currentChild.isEqualNode( futureChild ) ) {
		current.replaceChild( futureChild, currentChild );
	}
}

while ( current.childNodes[ i ] ) {
	current.removeChild( current.childNodes[ i ] );
}

[ellatrix]

This won't work I think. When you append or replace, the node will be gone from future. So either we have to remove unused nodes so that firstChild is accurate, or we copy all the nods to an array. Not sure if it makes much difference. These nodes are not live.

[aduth]

This won't work I think.

But it passes all tests, so it must work? 😄

So either we have to remove unused nodes so that firstChild is accurate, or we copy all the nods to an array.

There might be an option here with iterating backward from the end of future.childNodes as well.

Not sure if it makes much difference. These nodes are not live.

The theory (unconfirmed) is that removeChild is a non-trivial operation, even if not live in a document.

[ellatrix]

I think it’s a misconception that the DOM is slow, but I can make test.

[aduth]

I put together a benchmark. I'm a bit puzzled by how slow the normal iteration is, but it does show removeChild as being sufficiently performant.

https://jsbin.com/bemuhaj/4/edit?html,js,console,output

[ellatrix]

Yeah the DOM is pretty fast. Just not the live one... :)

[ellatrix]

I added an e2e test checking the selection change event and mutations which will fail on master. This hopefully prevents us from adding more of this kind of work on typing unintentionally.

[aduth]

This will be left as a lingering event listener, potentially called in a future test if the page is not reloaded before its assertions are run. Unlikely in practice; though generally a test should never leave any lingering side-effect.