View: Rendering and handling (in mutations) spaces at block boundaries and multiple spaces.

[Reinmar]

We need to figure out what to do with ckeditor/ckeditor5-design#61

Problems

TC1

Imagine that your model contains paragraph with "foo ". This needs to be rendered as <p>foo&nbsp;</p>.

TC2

User is typing at the end of a paragraph – "foo bar". We have "foo", then user presses space. That space must be rendered as   (we actually get   in a mutation). Then, two things may happen:

1. If user moved caret somewhere else and back to that place or we rerendered the selection, then browser may insert "b" without turning that nbsp into a normal space.
2. Typically (at least Chrome) converts that nbsp into normal space once "b" was typed. I checked that I can move selection around or load such data and the conversion takes place. I'm not sure however that it's going to work for all browsers and in all cases, so it's safe to assume that it won't.

Ideas

• A space typed at the end of a paragraph should be inserted as a normal space into the model. We may add a feature that Alt + Space (or whichever keystroke we'll choose from https://en.wikipedia.org/wiki/Non-breaking_space#Keyboard_entry_methods) will insert an nbsp to the model. Existence of such a feature shows that the model must always contain clean data.
• Multiple subsequent spaces must be rendered correctly.
• Mutations should rather contain un-quirked text content. So we assume that a space typed at the end of block, even though inserted as an nbsp by the browser should really be a normal space.
• However, the above should not be true for characters which were added as nbsps into the model and rendered as such. DOM converter must not convert them to normal spaces. This may be tricky ;/.

Alternative (temporary) workaround: we may for now use the CSS property which I described in ckeditor/ckeditor5-design#61 and do not care about this at all. The editor should work perfectly.

[pjasiun]

Makes sense. DOMConverter, together with Renderer and MutationObserver should handle it, so the View will be clear and beautiful. One day. ;)

[pjasiun]

TC 3

• editor with typing feature,
• model data: foo [bar] bom,
• press 'x'.

Expected result:
foo x[] bar

Actual result:
foo x[]bar

Reason:
What typing is doing is:

deleteContents( selectedContetn );
render();
handleMutations();
render();

After the deleteContents there is foo bom in the model. The two sibling spaces are rendered to the single space by the browser. Then, when mutation which handle insertion of the "x" character comes, it do the clean-up too and removes unneeded space, so there are two mutations: "remove space" and "insert x".

We should render one of these spaces as   to prevent it.

[scofalik]

Originally submitted in: https://github.com/ckeditor/ckeditor5-typing/issues/30

TC4

Similarly to TC1 and TC2:

Steps to reproduce:

• Write "Hello world"
• Remove "world"

What happens:
The word "world" is removed. However, the space that was between "Hello" and "world" is also removed.

TC5

Steps to reproduce:

• Write "Hello world"
• Remove "Hello"

What happens:
Both "Hello" and space between words are removed.

In both cases, space should be converted to   to ensure correct rendering.

[Reinmar]

OK, this issue is getting serious, because it's pretty evident and fails quickly.

To the cases described above I'd also add:

<heading1>Heading 1</heading1>
<heading2>[] </heading2>
<paragraph><$text italic="true">This</$text> is an <$text bold="true">editor</$text> instance.</paragraph>

The heading2 element is rendered with a normal space, so it's invisible.

[Reinmar]

Related issue: https://github.com/ckeditor/ckeditor5-engine/issues/606. Defines that spaces must be normalised when converting back to the model. I guess that both issues must be fixed together.

[scofalik]

Okay, let's summarize some knowledge that is spread around multiple issues and what we concluded:

1. We want to keep model clean. If user pressed space, we want breakable space in model. If user pressed shift + space we want   in the model. Multiple consecutive spaces should be just spaces in the model.
2. We would like to keep view clean aswell. We understand that it's perfect when view is as similar to DOM as it can be, but view is the entry point for some algorithms (mostly data processors) and we don't want to insert quirks into view. I see the need of normalize/replace characters as kind-of-a-quirk.
3. We'll try to make all the changes in the renderer. It would be better if we don't need to use white-space: pre-wrap; but we consider this solution as last restort.
4. Done
   Upon rendering from view to DOM, we need to:
   • change consecutive spaces to   as needed,
   • change spaces at boundaries to  
   • make sure that we are not leaving any unwanted  s. This will be done by not keeping unwanted  s in model or view -- only those created intentionally by user will be kept in view and model and they won't be removed.
5. We need to handle mutations for space key event:
   • shift + space inserts   into contenteditable, so we do not need to do anything, upon further investigation this turned out to be not true, it seems that there is no way to intentionally insert   in Chrome right now,
   • space may insert either space or  . Whenever &nbsp is inserted, we need to change it to normal space (and let model -> view -> DOM pipeline handle appropriate changes).
6. Done
   We need to handle setting contents (loading, pasting, etc.):
   • whitespaces (or any text) in wrong context should be removed, accordingly to rules set in Schema,
   • whitespaces in correct context should be handled, but.. how?

I think that most stuff that has to be done is clear but loading the content is not really, at least not for me. Let's say that somebody initializes editor with such html:

<div id="editor">
    <p>
        myText
    </p>
</div>

What about whitespaces inside <p>? How do we treat them? I guess we should remove all of them and treat this part of the code as <p>myText</p>. However, then, I am afraid we might have a conflict on how we treat whitespaces (see point 5.).

Keep in mind that I am not that familiar with Renderer and all code around it. I know what are the main goals and conceptions about it but still feel a bit lost. That's why I am not sure if it will be easy to handle pasting and typing in Renderer and related code differently.

[Reinmar]

I see the need of normalize/replace characters as kind-of-a-quirk.

I agree. Especially that it may depend on page styling. DOM renderer is the only bit which should be concerned by that.

We'll try to make all the changes in the renderer. It would be better if we don't need to use white-space: pre-wrap; but we consider this solution as last restort.

We always tried to not depend on certain page styling and not to force anything.

shift + space inserts   into contenteditable, so we do not need to do anything,

This may require a separate handler for Shift+Space because we won't be able to distinguish what was the user intention based on mutations.

whitespaces in correct context should be handled, but.. how?

They need to be normalised... Either upon conversion (view->model) or by the DOM converter (DOM->view). The latter won't handle spaces if e.g. Markdown DP was used, because then the DOM converter may not be used (MD->view->model).

What about whitespaces inside

? How do we treat them?

All should be removed.

However, then, I am afraid we might have a conflict on how we treat whitespaces (see point 5.).

Hmmm.... that's true.

[scofalik]

shift + space inserts   into contenteditable, so we do not need to do anything,

This is actually wrong, shift + space does not insert   :(

EDIT: In fact it might actually be better for us cause we can do it manually and we don't have to fight the browser.

[scofalik]

After some digging into the problem here is what I discovered and how I plan to solve this issue:

1. Enhance DomConverter#viewToDom so it takes care of converting spaces in view to &nbsps. This deals with point 4. above. I also changed Renderer#_updateText because it also takes data from ViewText#data and inserts directly into dom text node. Now, it first converts ViewText node to dom text node using DomConverter#viewToDom so all quirks that will be implemented in DomConverter will also be used in _updateText.
2. Pasting is now, obviously, not available, so for point 6. I focus on setting initial data. This is done through DataController which calls data processor, which then gets the data and converts it to view. We care about HTML and XML data processors. They both use DomConverter#domToView and DomConverter#viewToDom. I think its appropriate to modify domToView. Nothing else in engine right now uses this and I suppose nothing else in other repos. So it's safe to modify there. I will have to remove all unimportant whitespaces:
   • every double white space should be changed to single space (unless we are in preformatted block like <pre>),
   • space character at the beginning of block node should be removed - this is actually tricky part. It's tricky because at that point we do not know which view elements are blocks (ContainerElement) and which are inline (AttributeElement). We know it only after conversion from view to model and back. So I am not really sure how to solve this if we want to avoid putting this logic in view->model conversion (we need to check whether it's safe to put that logic there). After quick talk with @Reinmar we agreed that it's best to use hardcoded/configurable list of tag names that are treated as "container" elements. Spaces at the beginning of such elements will be removed. Note that we can't do this in view->model conversion because for some implementations additional spaces might actually be important and we can't just remove them.
3. Mutations operate directly on model using Batch API. The easiest place to modify is directly before batch.weakInsert() is called: https://github.com/ckeditor/ckeditor5-typing/blob/master/src/input.js#L234-L238
   Here we can just replace in text all instances of   with . Unfortunately it will be harded to implement shift + space behavior when we go this way. Another solution is to treat it like we did with enter. It has it's own event and does not rely on mutation. However, I suspect that mutations are used for a reason so I don't know if moving space key handling out of them is good idea. However, AFAICS, when handling mutations, we do not have information about pressed keys. So, AFAICS, it's impossible to check whether user had shift key pressed. This means that we probably have to manage this key combination like we do with`enter. We can keep normal space handling in mutations, though.

[pjasiun]

1 and 2 are fine for me.

But:

3 Mutations operate directly on model using Batch API.

No, they do not. They fire only the event which should have view data. For text there was not trasformation, because we did nothing with text. Now the text in the event fired from the mutation observer should be "view text".

[Reinmar]

Agree with @pjasiun.