RFC: Ensure emission of signals only after an operation is done, and only due to operations

[kossebau]

This is an alternative proposal for the problem first approached with #827:
implementations of operations emitting signals from the document before the operation has completed.

It is different from the first approach as it is more radical (in principle, exceptions needed still due to existing code):
no more can anyone emit a signal in the name of the OdtDocument whenever they want. Instead during the execution of an operation all events about changes on the document are collected, and when the execution has been completed, these events are all emitted in one go. Which also means that only by operation execution signals can be emitted.

Basic idea is:
only operations should modify the state of the document and thus events/signals be created

By encapsulating the operation execution and signal emission in code not influencable by the 3rd-party programmer, there is also no chance to do hacky emissions of signals.
emitSignalCursorMoved, emitSignalUndoStackChanged had to be added to deal with current hacks in the code, but hopefully in the future can be removed. fixCursorPositions also emits custom signals, yet to be solved.

Not yet sure if this is a sane approach and should be guiding the way into the future. So putting here mainly as a RFC for now, nothing I would like to see merged already yesterday. But then I hope latest March we found out what to do with this :)

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2327/

[peitschie]

This one and the undo stack modification definitely should be queued up when called via an operation. This one especially is one of the most frequent causes of the bug this patch is trying to fix.

I would instead suggest passing in the events array into function calls that need to generate signals, as there are a limited number of these.

For the specific edge case that is the shadow cursor, perhaps you can inject or expose a specialised callback to be used ONLY by the shadow cursor. This ensures operations don't get access to the these inappropriate methods.

Ah, right, I see that you've only moved the shadow cursor across to this. That means there are a bunch of yet-unfixed eventNotifier calls still in this class (e.g., moveCursor & fixCursorPositions) that fixCursorPositions still require fixing.

[peitschie]

Overall, I'm actually 100% ok with this. The key fixes I'd like to see are:

1. make any calls from an operation add to the operation's own event queue rather than firing directly.... without exceptions (otherwise, we defeat the key advantage this patch has over the previous attempt)
2. more clearly highlighting that the new emit functions on the document either shouldn't be used (e.g., prefix with DONOTUSE_ or something equally ugly to the eye), or (more preferred) actually find a way to prevent 3rd party components from getting to these completely somehow (e.g., constructor injection into the SessionController).

[kossebau]

Played with some code for getting all events added to operations queue. Not thrilled with the result yet, but possible a step into the right direction.
The undostack signal puzzles me some more. Actually not sure if that should be a signal that is emitted from the document. After all the undo stack is not a property of the document currently. And also not an event which is triggered by any operation. Interesting to see that the signal is routed from the undomanager by the sessioncontroller to the document, where then only EditorSession listens to it with the current Wodo classes. No good idea yet how that signalling should be resolved.
Same for the others. So for now just followed your proposal to prefix the method names with something ugly :)

[kogmbh-ci]

Build succeeded.
Refer to this link for build results: http://ci.kogmbh.com/jenkins/job/WebODF-PullReq/2382/