[Umbrella] Further widgets integration with ACF

[Marek Lewandowski]

Cases we need to solve

1. Integration with styles applicable to widgets. See #11297.
2. Disabling ACF in body of some widgets - e.g. mediaembed. See #11737 and #10925.
3. Filtering pasted widgets. See #11115.

Cause of problems

1. ACF filters data before widgets are upcasted. Therefore for example mediaembed cannot notify ACF that children of some elements should not be filtered and it's not possible to create allowed content rule based on a widget name and style properties allowed on that widget element.
2. In pasted or dropped content we have the upcasted and initialized HTML of a widget. Therefore we cannot filter it, because ACF is applied to the data format.

Solution

ACF after upcasting

The advantage of this solution is that ACF would know everything about widgets, so it could make precise decisions. Additionally, while filtering pasted content there would be no problem at all, because it would be the same case.

However, there are two problems which makes this idea incorrect:

• Assuming that we've got some complex widget like image2, which makes a lot of transformations while upcasting, how would ACF filters the upcasted image? How could it strip, without breaking widget, figcaption and figure if they are not allowed? It's simply impossible, without some special actions from the upcast method. It could theoretically ask ACF if specific components of widget are allowed, but it's tricky and still doesn't solve this issue completely.
• Moreover, when ACF would filter upcasted version of widgets it would be impossible or a lot harder to define ACF rules. One would have to know internal widget structure to define those rules. In my opinion this makes the idea totally wrong. ACF filters data format and that's a foundation we have to keep.

ACF filters data

Previous section proved that ACF needs to be applied to data, not to inner HTML. This means that the current way of processing is the only correct one, but on the other hand we still have those three cases, which are listed at the beginning, that have to be solved.

1. Integration with styles applicable to widgets - a solution has been proposed in ​http://dev.ckeditor.com/ticket/11297#comment:8. It may not be 100% precise in some cases, but there's another ACF's characteristic which we have to keep in mind. It's not meant to be a security filter. It's meant to ensure good UX in as many cases as possible. Therefore when uncommon constellation of element and its attributes slips through its hands it's not a blocker.

2. Disabling ACF without a possibility to communicate through element's attributes. In this case the only possible solution is a callback executed by ACF which will notify it about how it should proceed. For example mediaembed will add a callback which will tell ACF that all descendants of element should be skipped if element has mediaembed's class. Callback will return binary flags, because we may need more options in the future. Example of flags that we may need to implement:

• DISABLE_ON_ELEMENT - skips only this element (continues on descendants),
• DISABLE_ON_DESCENDANTS - skips descendants,
• CONTINUE_ITERATION - continue iteration - used with DISABLE_ON_DESCENDANTS could inform ACF that it should look for a descendant that reenables filtering,
• ENABLE - reenable filtering.

But of course for now we'll need just one of these flags, so only one should be implemented.

Ticket: #11737.

3. Filtering pasted widgets. It's not possible to filter widget's upcasted version - only its source version can be filtered. If we had full control over clipboard we could downcast selected data on cut/copy and then filtering on paste would work like filtering on data load. Unfortunately, we don't have full control over clipboard and won't have even after #11437, because of IEs.

Therefore, the only solution might be to look for upcasted widgets in pasted data, extract their HTML, find out from which editors they came from, use those editors to downcast them, and replace their upcasted version with downcasted one in pasted data. This could be based on unique editors' ids and data-cke-widget-editor-id attributes marking widget wrappers. This won't work when copying between window tabs or frames or when editor instance has been destroyed in the meantime and I'm not sure what should happen in such situation.

Similar solution, maybe more straightforward and more reliable would be if editor, on cut/copy/dragstart if it cannot override the content, could retrieve widgets' HTML from selection, downcast them and store in the CKEDITOR object, so all other editors will be able to replace pasted widgets with their downcasted HTML stored in that object. Of course widgets have to be marked with unique ids to make this possible. This option is better than the previous one because widget's HTML will be kept even after editor is destroyed. We can also reuse the same mechanism in drag and drop for IEs in which we will need to store the data for a while. As the mechanism will be public 3rd party code will be able to integrate with it.

And the third solution, nearly perfect, is to control copy and cut and override data with HTML extracted by us and processed by htmlDP.toDataFormat. We will need to do the same for drag&drop, so it won't be a big problem to eventually make the same with copy/cut. Additionally, we would automatically solve the problem with Blink putting awful HTML into clipboard.

[Olek Nowodziński]

cc

[Piotrek Koszuliński]

We've been discussing whether it wouldn't be much better if widgets were fully self-managing entities.

• They would be upcasted before content filtering and ACF would not touch them.
• They would filter their content during upcast. We realised that the only way it could happen is if upcast method would try to find all its parts inside the unfiltered structure and would output itself replacing the original content. For example image2 would have to find caption and image inside <figure style="ugly inline styles"><span><img>xxx</span><figcaption /></figure>, scan for any additional attributes/styles/classes it should keep and output clean HTML with only those HTML features which it handles.
• ACF would not control whether widget feature should be enabled - that would be controlled by enabled widgets.
• Theoretically there wouldn't be a problem with filtering pasted widgets because editor would know which widgets are enabled.

From one perspective this looks like a good design, because the control is in widgets' hands which know all about themselves and can make precise decisions how to handle excessive or missing content. In fact, it would be an implementation of a design proposed by me few months ago. Its assumption was that HTML is parsed to an intermediate form of a tree of features instances. Every enabled feature tries to find its HTML representation in the input and replaces it by its instance. The content filtering would be automatic, because every piece of HTML which wasn't replaced by feature would be rejected. In a later phase, while editing, features themselves would be controlling their representation in inner HTML, their states, etc. and they would output themselves to data. In this last phase we would be able to easily control whether we want inline styles or classes, because every style, every attribute will be kept in some feature's data property. Also the data format would be totally controllable.

I'm a big fan of this concept, but it's a completely new approach, very demanding in terms of architecture and in my opinion it only makes sense when implemented consistently throughout the entire editor. The widget case is good example of how hard would it be to implement this concept partially.

• Currently editor.activeFilter (an instance of ACF for the active editable) is requested when editor checks if a command should be enabled. If widgets' commands (i.e. features) would not be controlled by ACF but by enabled widgets list we would need it to be dynamic like the active filter and it would have to be configurable for nested editables too.
• Actually, we cannot assume that all registered widgets should be enabled, because one plugin may register more than one widget, so we would need a configuration option for enabling widgets.
• Upcasting widgets and some of their internal logic would have to be implemented in a totally different way than they are now and since widgets system is already working and there are plenty of widgets out there (ours and 3rd party) we cannot force such drastic change.
• Implementing a widget which could have multiple forms (e.g. a template) would be much harder, because for all of these forms one would need a builder which would scan input HTML, look for pieces that could be handled and build proper HTML from that. That's partially a duplication of ACF, so it could be reused, but neither it can be now, nor will it be easy and fast after necessary changes. Working on htmlParser.element's classes and inline styles (what has to be done because of integration with styles system) is hard because we serialize them. ACF parses them and keeps as separate properties so access is easy and fast, but they need to be serialized back to attributes. There are possible solutions for that, but they require making a lot of changes.

Of course, now ACF cannot precisely filter structures too. Or rather, it's not easy, because match option has to be used. But in the current situation I would rather learn ACF how to better filter structures (e.g. we can learn it about relations between rules), rather than duplicate its functionality in widgets.

• We're talking about very complex cases, but in fact current design works fine for the easy ones. If you've got a simple widget which doesn't use some complex upcast, it can define ACF rules quite precisely and the upcast is limited to discovering its special class or something. Integration with styles is based on a single property which defines widget element. And that's all. If widget was meant to handle all that then it would be a lot harder to implement even in such basic case.
• The integration with Drupal, which enables features through HTML elements list defined in the admin panel would not be possible any more or would be a lot more complex.
• One more aspect which we should consider is that making changes in APIs and whole concepts which we introduced not so long ago could not be well justified.

tl;dr Proposed idea is valid and makes a lot of sense in general. However, I think that it cannot be implemented now because it'd require a lot of changes in our and other code. I'm not also sure whether it's not too demanding for CKE5.

[Wim Leers]

This discussion is way over my head, but I just want to offer one piece of general advice:

[…] since widgets system is already working and there are plenty of widgets out there (ours and 3rd party) we cannot force such drastic change.

I think this may be acceptable, because:

1. Right now there are still few widgets.
2. Bad design choices now will then affect all widgets, instead of forcing the existing ones to be updated.

---

The integration with Drupal, which enables features through HTML elements list defined in the admin panel would not be possible any more or would be a lot more complex.

Can you explain why that would become impossible? Is it because each widget would then also need to be able to list its allowed HTML, which Drupal does not currently support? If that's it, I think we could make Drupal core only allow very basic tags in widgets by default (and make them overridable through configuration, but not through the UI), then a Drupal contrib module can come up with a UI to make it configurable.

[Piotrek Koszuliński]

I agree with the point that if a change is to be made, then better to make it quickly. But we won't make it in 4.4, because there's no enough time. Doing it later makes no sense, because it's too close to CKE5, which will introduce some other backward incompatible changes anyway. Better to make them at once.

Bad design choices now will then affect all widgets, instead of forcing the existing ones to be updated.

It's not clear if current design is bad. It's not perfect, but it allows to write simple widgets, well integrated with editor, with little effort.

---

Can you explain why that would become impossible? Is it because each widget would then also need to be able to list its allowed HTML, which Drupal does not currently support? If that's it, I think we could make Drupal core only allow very basic tags in widgets by default (and make them overridable through configuration, but not through the UI), then a Drupal contrib module can come up with a UI to make it configurable.

Yes, you're right. It can be solved somehow (so it's possible, but as you wrote - it'd more complex), but I just wanted to show that making widgets filter themselves will complicate things. The advantages are simply not worth the cost.

[Piotrek Koszuliński]

#11737 and #11297 are included in 4.4. #11115 has been postponed to 4.5 together with Drag and drop support. Therefore I'm moving this ticket to 4.5 too.

[Piotrek Koszuliński]

#11115 has been removed from 4.5.0 (see ​http://dev.ckeditor.com/ticket/11115#comment:8).

[Piotr Jasiun]

cc