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

New lines are not stripped together with other whitespaces #3986

Closed
Reinmar opened this issue Feb 19, 2017 · 5 comments · Fixed by ckeditor/ckeditor5-engine#988
Closed

New lines are not stripped together with other whitespaces #3986

Reinmar opened this issue Feb 19, 2017 · 5 comments · Fixed by ckeditor/ckeditor5-engine#988
Assignees
Labels
package:engine type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Feb 19, 2017

On conversion from DOM -> view we don't strip new line characters.

TC: Create an editor on the following contents:

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

In the second case you'll have a <paragraph>\n</paragraph> in the model.

@Reinmar
Copy link
Member Author

Reinmar commented Jun 20, 2017

I guess it's something in DomConverter#_processDataFromDomText(). It'd be good to take a look on that because those new paragraphs are disturbing when using the editor in real cases.

@Reinmar Reinmar self-assigned this Jun 28, 2017
@Reinmar
Copy link
Member Author

Reinmar commented Jun 28, 2017

OK... this is pretty bad. I wrote a couple of tests and most of them fail:

      ✖ new line at the end of content is ignored
      ✔ whitespaces at the end of content are ignored
      ✖ nbsp at the end of content is not ignored
      ✔ new line at the beginning of content is ignored
      ✔ whitespaces at the beginning of content are ignored
      ✖ nbsp at the beginning of content is not ignored
      ✖ new line between blocks is ignored
      ✔ whitespaces between blocks are ignored
      ✖ nbsp between blocks is not ignored
      ✖ new lines inside blocks are ignored
      ✔ whitespaces inside blocks are ignored
      ✔ nbsp inside blocks are not ignored
      ✖ all whitespaces together are ignored

@Reinmar
Copy link
Member Author

Reinmar commented Jun 28, 2017

This turned out to be really interesting.

Problems can be reproduced if there's just one whitespace between blocks or at the content boundary. But all works fine if there's more of them.

There's also a small problem that we don't support autoparagraphing &nbsp;s. They are simply being ignored. I guess we can live with that so I won't work on fixing this bit (unless I'll find just the right place :D).

@Reinmar
Copy link
Member Author

Reinmar commented Jun 28, 2017

OK, there's one more important aspect, right now we turn all kind of spaces (\s) to normal spaces and later we normalise those. This is unsafe. I can see in CKE4 we use ltrim() and rtrim() which are limitted to [ \t\n\r].

I think that we should do the same in CKE5 because we may cause data loss in some languages or with special spaces being used intentionally.

@Reinmar
Copy link
Member Author

Reinmar commented Jun 28, 2017

szymonkups referenced this issue in ckeditor/ckeditor5-engine Jun 30, 2017
Fix: Singular white spaces (new lines, tabs and carriage returns) will be ignored when loading data when used outside/between block elements. Closes #822.

Also, the range of characters which are being normalized during DOM to view conversion was reduced to `[ \n\t\r]` to avoid losing space characters (which matches `/\s/`) that could be significant.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 11 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:bug This issue reports a buggy (incorrect) behavior. 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:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
2 participants