Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Space normalization on to-model conversion #3827

Closed
Reinmar opened this issue Sep 21, 2016 · 2 comments
Closed

Space normalization on to-model conversion #3827

Reinmar opened this issue Sep 21, 2016 · 2 comments
Assignees
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Sep 21, 2016

As exemplified here: https://github.com/ckeditor/ckeditor5-markdown-gfm/issues/4#issuecomment-248614316

In the view produced by the DP we may have excessive whitespaces which should be removed when that content is being converted to the model.

Note: In the view, spaces which should be preserved must be a mix of  s and normal spaces. So in this case the view must reflect how they'd behave in the DOM. In the model, though, if we have a text node with multiple subsequent spaces we treat all of them as visible, so when converting to the view, they should be converted into a mix of  s and normal spaces. See https://github.com/ckeditor/ckeditor5-engine/issues/379.

This e.g. means that when typing multiple spaces, even though the browser will create mutations with either " " or " ", we'll insert normals spaces into the model. In order to support intentional non-breaking spaces, we must support Alt+Space keystroke.

@scofalik
Copy link
Contributor

scofalik commented Sep 28, 2016

This got moved from https://github.com/ckeditor/ckeditor5-markdown-gfm/issues/4 where it was a response to https://github.com/ckeditor/ckeditor5-markdown-gfm/issues/4#issuecomment-248614316

DP role is to take input data (string) and provide semantically correct ViewDocumentFragment that can be then converted to model.

Another DP role is to take input ViewDocumentFragment and provide output data (string) that can be stored (i.e. in database). Of course DP could produce <p>          Text</p> but I have a feeling that you meant that "DP role is creating valid view" [this was a direct answer to @fredck comment].

Anyway.

Let's see how it looks like in default editor for HTML:

  1. HTML string is converted by Data processor to DOM (using native DOMParser).
  2. DOM is converter by Data processor to ViewDocumentFragment (using DomConverter).

I haven't looked at MD data processor code, but I can imagine it has similar steps (something converts MD string to DOM and then we can use DomConverter to convert it to view).

So, let's get back to:

The DP role is creating valid HTML. <p>          Text</p> (with plain whitespace to be discarded) is definitely valid and the DP should not care about it.

Let's focus on HtmlDataProcessor, as given example is HTML. DP should be able to process given string, this is obvious. However, semantically, those whitespaces are to be ignored. Whitespace meaning "nothing" is a characteristic of HTML. In other language they might have a meaning, so we cannot arbitrary remove any chain of whitespaces at any time, because "they mean nothing". No, it's typical of HTML.

This means that we cannot do this during view -> model conversion. We need to do it sooner. That's why I've written above that DP is responsible for creating semantically correct view, because we want to convert correct view to the model and not care about clearing trash from the view.

So, to sum up, DP should be able to take <p>          Text</p> but it should create correct view, without spaces. It also should return correct HTML. HTML created from view created from <p>          Text</p> is <p>Text</p>, though.

Unless we have a data processor with an option to beatuify the code :P then it can add some whitespaces :P

@Reinmar
Copy link
Member Author

Reinmar commented Oct 17, 2016

Already done.

@Reinmar Reinmar closed this as completed Oct 17, 2016
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 4 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

No branches or pull requests

3 participants