Selection change:attribute and change:range are fired far too often

[Reinmar]

They are fired multiple times whatever you do. This makes them useless, because even if e.g. attributes don't really change, the event is fired, so https://github.com/ckeditor/ckeditor5-typing/issues/21 cannot use this event.

This issue is also related to https://github.com/ckeditor/ckeditor5-engine/issues/259.

[pjasiun]

I think it is not related to #3597. It seems to be a dup of #3597.

[scofalik]

What we know is that selection events are mess, so this is not a matter of specific issue, it should be re-thought as a whole.

One thing is how this is solved in model. Now we know it is wrong that change:attribute is fired even when attribute has not really changed -- it is in fact changed each time we fire change:range. We should not fire change:attribute if the attribute key is same. Maybe we should not even fire LiveSelection#_updateAttributes for every change:range. For example, we might not fire this function if selection parent is same, but it gets complicated when you think of multiple ranges selection.

The other thing is how often model selection events are fired because of DOM/view. On one hand it seems wrong that change:range is fired after each letter is typed. OTOH, however, features may expect that, because, all in all, selection has actually changed. Unfortunately, IDK how DOM<->view<->model selection update works in details. Maybe DOM selection is changed too often? We know that we have so-called "selection infinite loops". Maybe we fire too many view selection changes?

I'd not only review model selection implementation but also DOM and view selection conversion/handling. What I know is that typing a letter should fire just one change:range event at most.

[maxbarnas]

While working on https://github.com/ckeditor/ckeditor5-typing/issues/21 I made prototype of a fix. The thing that was needed was a listener for changesDone in ckeditor5-typing/input.js: ckeditor/ckeditor5-typing@25e0429#diff-8052e4286a369903aa5d7f6c2d8b7190R49.

Without this I was unable to wrap all change:attribute events with my lock mechanism in changebuffer.js. I noticed changesDone handler in https://github.com/ckeditor/ckeditor5-engine/blob/master/src/model/document.js#L112 and used it with success. But that solution is far from being clean.

This makes handling all change:attribute events impossible from input.js and needs fix.

[maxbarnas]

Also what do you thing about creating new, more general events to help with situation described in this issue?

[Reinmar]

I think it is not related to #3597. It seems to be a dup of #3597.

I don't think that it's a DUP. This issue is not only about resolving the way how and when selection attrs are updated, but rather about how it works inside the selection and the amount of noise that the events currently cause. E.g., as far as I understand, if you change sel attrs from "x:true,y:true" to ... "x:true,y:true", you'll get the events.

Anyway, we need to debug this and plan this from two perspectives – correctness (#259) and the number of events and their usefulness (this ticket).

E.g. see @maxbarnas's comment – the events now are fired too late, so you can't reason about them.

[Reinmar]

Also what do you thing about creating new, more granular events to help with situation described in this issue?

I think it's the opposite :D. We need less noise, but more precise and useful events. So we shouldn't fire just to fire things. We should fire events to solve specific use cases. This is one of them – checking if selection attributes changed so we can reset the buffer. But at the same time, we want to filter changes done using that buffer.

[maxbarnas]

@Reinmar I agree and I corrected myself while you were writing this comment :)

[Reinmar]

ckeditor/ckeditor5-engine#634