This is the root cause of bug 54335. Basically, you open the language inspector (or the link inspector, for that matter), make a change, then close the inspector by clicking elsewhere into the document (as opposed to by using the arrow or by pressing enter or escape in the link inspector's text input).

Narrated call stack:

• A mouseup event fires on the document
• ve.ce.Surface.onDocumentMouseUp() starts the observer and polls
• The observer notices that the selection has changed and emits a selectionChange event, which ends up invoking ve.ce.Surface.onSelectionChange
• onSelectionChange sets a render lock and changes the model selection
• ve.dm.Surface.change() emits a change event
• ve.ui.Context.onChange() notices that the selection changed while an inspector was visible, so it closes the inspector
• ve.ui.AnnotationInspector.onClose() saves the changes the user made to the model, by indirectly calling ve.dm.Surface.change()
• (at this point, we have a change() call stack frame nested inside another change() frame, which is always a bad sign)
• The transaction processed by onClose() annotates text, which causes an update event to be emitted
• ve.ce.ContentBranchNode.onChildUpdate() responds to this event and calls renderContents()
• renderContents() checks to see if the surface is locked for rendering; it is, so it bails and doesn't render the change

When I briefly talked to Trevor about this issue, he said something about emitting an event asynchronously. I dismissed it at the time, because it would just move both problems (having to lock to prevent the model normalizing the selection / event storms, but having to not lock to allow inspector changes to render), but now I think about it I think it has merit. We could have ve.ui.Context.onChange() asynchronously close the inspector, from a setTimeout(). That would avoid the nested change() thing, and it would allow the render lock to be lifted before the inspector is closed.

Alternatively, onSelectionChange could only lock against selection changes and still allow transactions. But the nested change() seems like a code smell anyway, the order of event handlers might get messed up for instance, and the caller would observe multiple changes from calling change() once (that's the root of the problem here).

Version: unspecified
Severity: normal