Ckeditor5: Impossible to listen for attribute change on model's root

[mlewand]

📝 Provide detailed reproduction steps (if any)

We had a concept where we wanted to store some data (here keywords) as a single item in the model. An idea was to simply put it as an attribute on the model root and convert it into a element (only) in data downcast pipeline (it's not needed for editing purpose).

However it turned out that the downcast dispatcher doesn't fire attribute event for attributes on $root.

You can see it using the demo: https://codepen.io/mlewand/pen/wvWOZxW?editors=1010

✔️ Expected result

Referenced code should console log "All attr change listener: \" also for $root (the attribute is properly placed on $root by upcast converter).

❌ Actual result

The only "All attr change listener: \"  log is triggered for the bold attribute on text.

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[scofalik]

Data pipeline

First, let's discuss data pipeline, that is the pipeline used when called editor.getData() that goes: model -> view -> HTML output.

The problem isn't that events aren't fired for $root element. The problem is that the $root element is not converted at all.

The suggested solution for this issue is to change how DataController#toView() works. At the moment, the method gets an element or document fragment as a parameter and it should stringify it. However, in fact, it does not stringify the element but instead, it stringifies all the children of that element/document fragment.

Instead of doing that, we could convert the given element. How does that change the output generated from toView()?

1. DocumentFragments shouldn't have any conversion, so there's no difference whether we convert it or its children.
2. $root shouldn't have any conversion as well. I would be very highly surprised if any of our users defined conversion for either DocumentFragment or $root as it not only does not work (for $root in data pipeline) as well as it might actually brake things (for DocumentFragment).
3. Other elements. Now, this is the tricky part. Users (or maybe even our code or features :thinking: not sure here) might actually use DataController#toView() on elements for some reason. This makes sense and I wouldn't be surprised if someone used it that way.

Keeping all of this in mind, we could change the behavior of toView() only for $root elements. So, if the passed element is a root (use Element#is to check it), we don't create range in it, but on it. I don't think this is something ugly, it makes sense. If we allow for adding attributes for the $root element we should allow converting them (duh) and this is the way to do it.

However, in my opinion, the way how toView() works for elements is incorrect. If I pass an element there, I want the whole element to be converted, not only its children. If I pass <paragraph>Foo</paragraph> there, I want to get <p>Foo</p> not Foo. In my opinion, the output at this moment is unexpected and confusing. Actually, getting <p>Foo</p> is difficult now. I need to wrap that model paragraph into an artificial document fragment. This is silly.

So, honestly, if I were to decide, I'd do this:

1. Change the behavior of toView() for everything, not only for $root.
2. Check if tests fail. Check which tests fail and why. Check if there's a lot of places where we use toView() for non-root elements.
3. If no, go with the breaking change. If anyone uses toView() for non-root elements, handling this breaking change should be relatively easy.
4. We could also add a possibility to pass a range to toView() (so you can pass element, document fragment or a range). Then handling that breaking change will be very easy if someone used toView() in their custom code.

After that change, in the data pipeline, addAttribute should be fired for $root.

By the way, AFAICS, this change should also fix this: #8485 :).

Editing pipeline

Editing pipeline (model -> view -> editable element) is a different story. If we want to reflect root attribute changes, we need Differ to process them, which doesn't happen at the moment.

In Differ there's bufferOperation() which will need to handle incoming RootAttributeOperations. It will be easier to handle than attribute operation because those operations can be only on a root (so there are no intersecting changes that you need to handle). So basically:

1. Root attribute operation comes in.
2. You check on which root it operates and what attribute it changes.
3. You updated the state. For example, the operation may overwrite another operation that was just handled.
4. Then you need to return those changes. We can return them in getChanges() or if it is easier we can return them using a new method (getRootAttributeChanges()).

Summary

This task can be split into two parts: fixing data pipeline and fixing editing pipeline.

I believe that changes described in this ticket are important for one reason: it seems to me that root attribute operation is a feature that never worked, it cannot be used because other parts of the system do not support it. It's probably not used anywhere and we have a dead code. It seems that there are use cases for root attribute though, so... let's fix it?

[niegowski]

So, if the passed element is a root (use Element#is to check it), we don’t create range in it, but on it.

It's impossible to create Range on $root element, it can be only 'in' it. 

Also for the current usage of DataController#toView or DataController#stringify on an element that is not a root element - I found only one: https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-heading/src/title.js#L127-L132

[niegowski]

But maybe we could implement sth like RootRange that would extend Range, internally it would be Range inside the element but it would extend 

* [ Symbol.iterator ]() {
    yield* new TreeWalker( { boundaries: this, ignoreElementEnd: true } );
}

With the $root element itself?

[jodator]

Side idea: Maybe use some named, side document root.

So instead of hacking it in the current $root we could store this data outside main root in some, always available $special root.

[scofalik]

We can always try a solution to add the $root to a temporary document fragment and make a range there.

[mlewand]

I'll just mention that workaround suggested by @jodator worked for our use case. That being said it would be nice to fix this issue at some point.