Empty lines are removed in enter mode BR

[Jakub Ś]

To reproduce:

1. Open enterkey sample and set enter mode to BR
2. in WYSIWYG press new page command and then press Enter key few times. You will get:

   <br />
   <br />
   <br />
   

3. Toggle between WYSIWYG and Source

Result: Every time you switch one BR gets removed until there is nothing in editor. This is in CKEditor 4 beta

4. From CKEditor 4.0 you will get something like below when switching to source.

   <br />
   <br />
   <br />
   &nbsp;
   

5. Every time you switch to source remove the  . Only then br tag gets remove (one at a time)

Problem can be reproduced in all browsers from CKEditor 4 beta. It doesn't occur in 3.x branch.

[Jakub Ś]

#10145 was marked as duplicate.

[Jakub Ś]

#10246 was marked as duplicate.

TC:
Paste below:

    <br /> 
    <br /> 
<br /> 
    <br /> 
    <p>This is a paragraph of text.</p> 
    <br /> 
    <br /> 
<br /> 
    <br /> 
    <p>Second paragraph of text.</p>
<br /> 
    <br /> 
    <p>Second paragraph of text.</p>

You get

<br />
<br />
<br />
&nbsp;
<p>This is a paragraph of text.</p>
<br />
<br />
<br />
&nbsp;
<p>Second paragraph of text.</p>
&nbsp;

<p>Second paragraph of text.</p>

This particular TC can be reproduced from CKEditor 4.0 but ths is because there is something behind br tag. If there was nothing result would be the same as in 4.0 beta.

[Jeremy Jannotta]

Hi, is there an ETA on fixing this issue? It's actually a significant blocker for us upgrading to v4 (from 3.5.1), and would love to have a fix, or at least a workaround, so we can upgrade as soon as possible.

[Jakub Ś]

Currently there is neither ETA nor workaround for this issue. I will however ask my colleagues about this.

The only let’s say good news is that after first change   seem to block further 'br' removal.

UPDATE: Currently there is no eta for this ticket.

[Piotrek Koszuliński]

I bisected the repo until the beginning of v4, but it is reproducible even then, so I'm not able to find a commit which has broken this.

[Jeremy Jannotta]

Thanks for looking into this guys.

If we had to as a short-term workaround, can you think of a way to preserve the line breaks after they are input, such as on a 'dataReady' event, or using htmlDataProcessor, or the like?

Also, to clarify, even though the results look mostly the same, we're seeing all the br tags removed in this scenario, not just the last, so:

<br>
<br>
<p>This is a paragraph of text.</p>

ends up like

 
<p>This is a paragraph of text.</p>

[Jakub Ś]

I think there is a workaround for this issue. Its kind of hackish but result is rather consistent.

var editor = CKEDITOR.replace( 'editor1', {
	
				enterMode : CKEDITOR.ENTER_BR
			} );
			
			editor.on('pluginsLoaded' , function( evt ){ //it must be done before "instanceReady" to make the tag blocking in effect
						var counter = 0;
						evt.editor.dataProcessor.dataFilter.addRules({
							elements :{
								br : function( element ) {
									if(element.next.value == ' ' || element.next.name == 'br'){										
										counter++;
										return;										
									}
									
									if(counter <= 2){
										var helper = new CKEDITOR.htmlParser.text('&nbsp;');									
										helper.insertAfter(element);
									}
									counter = 0;			
								}
							}
						});
					});

The above code

//for:
<br>
<br>
<p>This is a paragraph of text.</p>
//and:
<br>
<p>This is a paragraph of text.</p>

will result in:
<br />
&nbsp;
<p>This is a paragraph of text.</p>


and for:
<br>
<br>
<br>
<br>
<p>This is a paragraph of text.</p>

will result in:

<br>
<br>
<br>
&nbsp;
<p>This is a paragraph of text.</p>

[Jeremy Jannotta]

Replying to j.swiderski:

Thanks! This helps a lot, and and gets us so much closer.

I noticed a couple of things:

• Changing counter <= 2 to counter <= 1 , fixes an issue with 3 BR tags resulting in 3 BRs and a NBSP (vs 2 BRS and a NBSP).
• Have been playing around with this code, no luck so far, to find a way to preserve a single line break (with this addition, one BR results in BR and a NBSP, this two line breaks). Not as big a deal, but would like to preserve as much of the original formatting as possible. One BR resulting in one NBSP would be ok in this case.

I think there is a workaround for this issue. Its kind of hackish but result is rather consistent.

var editor = CKEDITOR.replace( 'editor1', {
	
				enterMode : CKEDITOR.ENTER_BR
			} );
			
			editor.on('pluginsLoaded' , function( evt ){ //it must be done before "instanceReady" to make the tag blocking in effect
						var counter = 0;
						evt.editor.dataProcessor.dataFilter.addRules({
							elements :{
								br : function( element ) {
									if(element.next.value == ' ' || element.next.name == 'br'){										
										counter++;
										return;										
									}
									
									if(counter <= 2){
										var helper = new CKEDITOR.htmlParser.text('&nbsp;');									
										helper.insertAfter(element);
									}
									counter = 0;			
								}
							}
						});
					});

The above code

//for:
<br>
<br>
<p>This is a paragraph of text.</p>
//and:
<br>
<p>This is a paragraph of text.</p>

will result in:
<br />
&nbsp;
<p>This is a paragraph of text.</p>


and for:
<br>
<br>
<br>
<br>
<p>This is a paragraph of text.</p>

will result in:

<br>
<br>
<br>
&nbsp;
<p>This is a paragraph of text.</p>

[Matthew Leffler]

@j.swiderski, Can we get this targeted to a milestone? This is an annoying bug for us. :)

[Jeremy Jannotta]

I second that, getting this on the roadmap would be great. Despite the workaround we have in place, which feels more like a tenuous band-aid, it's still not preserving the original formatting which is intended.

Replying to mattleff:

@j.swiderski, Can we get this targeted to a milestone? This is an annoying bug for us. :)

[Jakub Ś]

I have informed my supervisor about this issue. I haven't received information about any milestone but now our senior developers know about this issue so there is a chance they will have it fixed in one of future releases.

[Chris Wells]

Just adding myself to the CC here as this issue is keeping me from updating. @j.swiderski and I are trying to determine whether my submission #10381 is a duplicate of this one.

[Jakub Ś]

#10381 was marked as duplicate.

Workaround used here doesn't quite work for #10381 example. Perhaps it will be possible to update this workaround.

[Chris Wells]

Below is an updated workaround that handles my issue as well. Unfortunately I couldn't find a way to improve on the previous code or combine it with mine better. I may be able to spend some free time on this later, but for now I've maxed out my work time for it (:

editor.on('pluginsLoaded' , function( evt ){ //it must be done before "instanceReady" to make the tag blocking in effect
  var previousNext = false;
  evt.editor.dataProcessor.htmlFilter.addRules({
    elements :{
      $ : function( element ) {
        if (element.previous || element.next) {
          if (previousNext && previousNext.value == ' ' && element.previous && element.previous.name =='br') {
            var helper = new CKEDITOR.htmlParser.text('<br>');                  
            helper.insertBefore(element);
            previousNext.remove();
          }
          previousNext = element.next;
        }
      }
    }
  });
  
  var counter = 0;
  evt.editor.dataProcessor.dataFilter.addRules({
    elements :{
      br : function( element ) {
        if(element.next && (element.next.value == '&nbsp;' || element.next.name == 'br')){                    
          counter++;
          return;                    
        }
            
        if(counter <= 1){
          var helper = new CKEDITOR.htmlParser.text('&nbsp;');                  
          helper.insertAfter(element);
        }
        
        counter = 0;      
      }
    }
  });
});

With this code...

<table>
	<tbody>
		<tr>
			<td>Table cell contents</td>
		</tr>
	</tbody>
</table>
<br>
<p>P contents</p>

will result in...

<table>
	<tbody>
		<tr>
			<td>Table cell contents</td>
		</tr>
	</tbody>
</table>
<br />
&nbsp;
<p>P contents</p>

[Frederico Caldeira Knabben]

Let's try to have it in the 4.1.2.

[Olek Nowodziński]

At the moment the following can be confirmed in ENTER_BR:

• <br /> tags disappear one-by-one when switching WYSIWYG -> source -> WYSIWYG-> etc. only in Firefox. This can be easily fixed by disabling ​this replace in ENTER_BR.

• Additionally, in all browsers, the last <br /> is recognized as a bogus and cleanBogus() removes it when htmlDataProcessor outputs data (toDataFormat). brFilter() appends   in this place so for this code in WYSIWYG:

  foo<br /><br /><br />
  

  editor.getData() returns:

  foo<br /><br />\uA0
  

[Frederico Caldeira Knabben]

Replying to a.nowodzinski:

• <br /> tags disappear one-by-one when switching WYSIWYG -> source -> WYSIWYG-> etc. only in Firefox. This can be easily fixed by disabling ​this replace in ENTER_BR.

Yes, that's a bug.

• Additionally, in all browsers, the last <br /> is recognized as a bogus and cleanBogus() removes it when htmlDataProcessor outputs data (toDataFormat). brFilter() appends   in this place so for this code in WYSIWYG:

  foo<br /><br /><br />
  

  editor.getData() returns:

  foo<br /><br />\uA0
  

That's a feature and it's there to guarantee consistent rendering in all browsers. On editing the &nbsp; is reverted to <br>.

[Olek Nowodziński]

Created branches t/10146 in ​dev and tests-v4.

• Proposed a simple fix for unnecessary bogus removal in Firefox.
• Created TT for the ticket.
• Created MT for the ticket.

[Frederico Caldeira Knabben]

The mt is not needed. Ok, for the rest. It doesn't seem to bring regressions.

[Olek Nowodziński]

Merged fix into master, dev (​​git:89d740763d), tests (c6ce5c800db385).

[Chris Wells]

Hello folks, pardon my ignorance, but what happens with the additional bugs that were pointed out in the comments of this ticket? I have tested with the changes and see the improvement in Firefox, however my original ticket (#10381) that was marked as a duplicate of this one is not affected, nor is the scenario from jpj109's comment (from #10246, which was also marked as a duplicate of this issue).

[Jakub Ś]

@cwells I have checked these issues you describe. In this case you would need to use web-tools to check editor behaviour :).

In WYSIWYG this is being displayed as br while in Source mode as &nbsp;. Current fix uses such transformations br->&nbsp;->br.

The only problem I have noticed is with:

<br>
<br>
<p>This is a paragraph of text.</p>

result for it is the same as for

<br>
<p>This is a paragraph of text.</p>

Both are changed in source mode into

 
<p>This is a paragraph of text.</p>

Which means that for two br's one is permanently removed. Other br quantities (1,3,4...) are handled correctly.

[Chris Wells]

Replying to j.swiderski:

@cwells I have checked these issues you describe. In this case you would need to use web-tools to check editor behaviour :).

Thanks for sticking with me on this. Could you elaborate on what you mean by using web-tools to check editor behavior? I'm sorry to be so adamant about this issue, it just happens that it affects a lot of places in our website that have <table>, <hX> and <br> elements that just won't play nice together in ckeditor4.

[Jakub Ś]

I have meant tools like Firebug, Chrome web-tools or IE dev-tools.
The point is, you see in Source mode &nbsp; and in WYSIWYG br.

If you get data from editor e.g. with getData method you will see it contains BR. Please try your code with ​http://nightly.ckeditor.com/13-05-29-13-05/full/samples/api.html. You will see br is alerted.

[Chris Wells]

Replying to j.swiderski:

I have meant tools like Firebug, Chrome web-tools or IE dev-tools.
The point is, you see in Source mode &nbsp; and in WYSIWYG br.

If you get data from editor e.g. with getData method you will see it contains BR. Please try your code with ​http://nightly.ckeditor.com/13-05-29-13-05/full/samples/api.html. You will see br is alerted.

Ok, understood. In my testing, however, I am not getting the <br> in getData. With Chrome's developer tools, here is what I get using a local copy of ckeditor-dev-master/plugins/enterkey/samples/enterkey.html:

I enter the following code in source mode:

<table>
	<tbody>
		<tr>
			<td>Table cell contents</td>
		</tr>
	</tbody>
</table>
<br>
<p>P contents</p>

and then change to WYSIWYG and run CKEDITOR.instances.editor1.getData() in console...

This is the result in ENTER_P mode:

<table>
	<tbody>
		<tr>
			<td>Table cell contents</td>
		</tr>
	</tbody>
</table>

<p>&nbsp;</p>

<p>P contents</p>

<p>&nbsp;</p>

This is the result in ENTER_BR mode:

<table>
	<tbody>
		<tr>
			<td>Table cell contents</td>
		</tr>
	</tbody>
</table>
&nbsp;

<p>P contents</p>

[Jakub Ś]

Problem with enter mode P has already been described here - ​http://dev.ckeditor.com/ticket/10431.

About Enter mode BR – I’m sorry for my last comment. Actually what you see in source mode is rather the same thing you will get with getData method.
The behaviour should rather be the opposite - &nbsp; in source and br in WYSIWYG.

I will address this ticket to my colleague as now I simply don't know why it works that way. Perhaps there is no other way then presented in workaround and last BR has to changed to &nbsp;

[Jeremy Jannotta]

Thanks for all your work looking into this, but I was wondering if there is any progress yet on solving my original test case?.

Release 4.1.2 does not fix it, and as commented in ​http://dev.ckeditor.com/ticket/10146#comment:26 , changing two <br>'s to a single &nbsp; is still a major issue at the very least.

Because this issue still exists, I vote we reopen this ticket/create a new ticket, as this is still a big problem.

[Jakub Ś]

Replying to comment:31:
Original issue is ​http://dev.ckeditor.com/ticket/10146#comment:9 and yes it hasn't been fixed in IE, Opera and Webkit (only FF seems to work fine).

[Jakub Ś]

#11308 was marked as duplicate.

This issue is continued in #11392.

[Jakub Ś]

var editor = CKEDITOR.replace( 'editor1', {
	enterMode : CKEDITOR.ENTER_BR
});	

editor.on( 'pluginsLoaded', function( evt ){
	evt.editor.dataProcessor.dataFilter.addRules({
		elements :{
			br : function( element ) {			
				//if next element is BR or <!--cke_br_comment-->, ignore it.
				if( element && element.next && ( element.next.name == 'br' || element.next.value == 'cke_br_comment' ) ){
					return;
				}else {
					var comment = new CKEDITOR.htmlParser.comment( 'cke_br_comment' );
					comment.insertAfter( element );	
				}
			}
		}
	});
				
	evt.editor.dataProcessor.dataFilter.addRules({
		elements :{
			li : function( element ) {
				if( element && element.next && element.next.name == 'br' ){
					var nextElem = element.next;		
					while( nextElem.next && nextElem.next.name == 'br' ){
						nextElem = nextElem.next;
					}
					var comment = new CKEDITOR.htmlParser.comment( 'cke_br_comment' );
					comment.insertAfter( nextElem );	
				}
			}
		}
	}, 1 );//priority defaults to 10
				
	evt.editor.dataProcessor.htmlFilter.addRules({
		comment : function( value, node ) {
			if( value.indexOf('cke_br_comment') >= 0 ) {
				return false;
			}
		}
	});
				
});

Above is the workaround which works for all test cases mentioned here except one:

<br>
<br>
<p>This is a paragraph of text.</p>

For some reason br br element is always changed to br element