[Webkit] Space after filling char should be secured

[Piotrek Koszuliński]

Based on: ​https://github.com/ckeditor/ckeditor-dev/pull/24

1. Open any sample.
2. Write text.
3. Press shift+enter.
4. Press space and type text.
5. Switch to source mode and back - space after <br> is lost.

Note: filling char is also removed in selection.js:490 and perhaps this place should be fixed too.

[Piotrek Koszuliński]

I'm adding this ticket to 4.1.1 milestone conditionally - if the patch is good (or doesn't need a lot of additional work), let's use it. If not, then we won't be able work on it for 4.1.1.

[Jakub Ś]

Please note that Webkit browser inserts blank space at the beginning of text and not &bnsp; like at the end. Are you sure this is good idea to "fight" with browser default behaviour?

NOTE: Perform this TC once and you get desired result. Now press "New Page" command (don't refresh page) and do it one more time. Notice that this time space isn't lost. Safari shows blank space, Chrome however shows $#8203; which makes me think that working it in second case is caused by #10031 in both (only Safari doesn't show this character)

[Piotrek Koszuliński]

Pushed ​t/10291 on dev.

[Piotrek Koszuliński]

Coincidentally I was working on two tickets that touches filling char subject at the same time and it'd be safer to test t/10291 on t/10315b (#10315) which fixes filling char removing after new page / switching to source and back.

[Olek Nowodziński]

This patch totally fixes the case and, if there's no other solution, it can be safely accepted. However there's another case that one that makes some trouble:

1. Set data:

   <p>foo^ bar</p>
   

2. Shift+Enter
3. editor.getData()
   1. Expected:

      <p>foo
      &nbsp;bar</p>
      

   2. Actual:

      <p>foo
      bar</p>
      

The space before bar is lost and I can't find any solution for this. If there's no easy solution, then we can accept t/10291 as at the moment this is enough.

[Piotrek Koszuliński]

The reason why this patch does not cover getData() (despite the fact that replace function is used for getData() as well) is that the following space is a separate text node.

We could of course cover also this case, but that would make this code even more complex. Or even extremely complex, because we should replace this nbsp with normal space after someone typed something before it. Unfortunately I checked that Chrome will not do that for us.

	var next = fillingChar.getNext();
	// Next text node is empty. Find first which is not.
	while ( next.type == CKEDITOR.NODE_TEXT && !next.getText() )
		next = next.getNext();

	if ( next.type == CKEDITOR.NODE_TEXT && next.getText().indexOf( ' ' ) == 0 )
		next.setText( next.getText().replace( /^ /, '\xa0' ) );

Then after pressing shift+enter start typing and nbsp will remain between new text and "bar".

Therefore, we're not able to fix this right now. In fact, I think that we should take different approach - instead of playing with DOM (what's very complicated and unsafe) we should replace filling char after getting HTML (when doing snapshot or getting data). I'm pretty sure that we can safely remove all \u200b characters fixing also preceding and following spaces. Although, I'm not confident about snapshots and corresponding bookmarks. This needs to be investigated.

So for now, let's have this simple fix and we'll work on the rest later.

[Piotrek Koszuliński]

Fixed on master with ​git:12de140.

I extracted idea described by me in comment 7 to #10332.