Always check selection range when creating undo levels

[johngodley]

Occasionally I see an error being thrown when clicking around a rich text editor:

I'm not entirely sure what triggers it, but I've got the steps down to a vague:

1. Quickly create a bunch of paragraphs
2. Click inside the paragraph blocks, but not on the text
3. Maybe get an error

Not the best steps, but hopefully this shows it better:

The problem seems to be a mousedown event triggering an undo history event, and the window.getSelection() not having any range.

In this PR I've added a check to window.getSelection() so it looks at rangeCount, as well as adds checks for all uses of createRecord.

I'm not entirely sure if this is the best solution, but it fixes the problem at hand. The error may indicate another problem I'm unaware of (with a better solution!)

The error only happens for me in Chrome (tested with 70.0.3538.77). It doesn't happen in Firefox 63 or Safari 12. The error is triggered more easily with the Unified Toolbar option enabled, although I've had it happen without.

Using Gutenberg master

Types of changes

Additional checks added for selection range in rich text editor

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]

The input event implies that there is user input. We cannot simply ignore this input and save nothing.

[ellatrix]

It might be better to still create the value, but not pass a range in this case.

[ellatrix]

I would really love to see reproducible steps so I know exactly what is causing the issue. Not setting the right selection may cause bugs that will be hard to track down.

[johngodley]

Good suggestion on the range. I've changed this to pass null, and removed all the extra checks. I don't know if this will have any negative effect, but in testing it doesn't seem to. However, I'm very unaware of where the value goes after this point.

I still haven't been able to nail down exactly what the conditions are for causing the error, but it is consistent if you follow the steps I put originally.

I suspect it may be a TinyMCE thing as I also noticed that normally when I click around onSelectionChange is fired with mousedown. In the case of the empty range the mousedown event comes through onCreateUndoLevel, via TinyMCE's change handler.

I don't know if this helps in anyway.

Looking at the docs for Selection it does seem that an empty Selection (type None) is possible, so it probably makes sense to check for this.

[ellatrix]

This makes sense to me.