Using justify in BR mode doesn't remove trailing BR's in IE thus resulting in new line

[Jakub Ś]

Steps to reproduce

1. Configure CKEditor so that it uses (you can also use allowedContent:true for simplicity)

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

2. Switch to source mode and paste below HTML:

   <div >
   	<div >
   		<div style="font-family: tahoma,arial; font-size: 9pt;">
   			<div  style="padding: 4px;">
   				<div  >
   					<span style="border: 0pt rgb(0, 0, 0); text-align: left; color: rgb(0, 0, 0); padding-top: 0px; padding-bottom: 0px; font-style: normal; font-weight: normal; text-decoration: none; margin-top: 0px; margin-bottom: 0px;">
   						<span style="padding-top: 0px; padding-bottom: 0px; margin-top: 0px; margin-bottom: 0px;">
   							<span class="ddspellcheck" id="scayt3">jkhgkjh</span>
   						</span>
   						<br>
   						<span style="padding-top: 0px; padding-bottom: 0px; margin-top: 0px; margin-bottom: 0px;">
   							<span class="ddspellcheck" id="scayt2">gkjhgkjh</span>
   						</span>
   						<br>
   						<span style="padding-top: 0px; padding-bottom: 0px; margin-top: 0px; margin-bottom: 0px;">
   							<span class="ddspellcheck" id="scayt1">gkjhlgfkhj</span>
   						</span>
   					</span>
   					<br>
   				</div>
   			</div>
   		</div>
   	</div>
   </div>
   

3. Switch to wysiwyg, click inside middle line and click justify center.

Expected result

Middle line gets centered.

Actual result

Middle line gets centred but extra line is added. This is happening because BR in first span (which gets wrapped into div) doesn't get removed like in modern browsers.

<div>
	<div>
		<div style="font-family: tahoma,arial; font-size: 9pt;">
			<div style="padding: 4px;">
				<div>
					<span style="border: 0pt rgb(0, 0, 0); text-align: left; color: rgb(0, 0, 0); padding-top: 0px; padding-bottom: 0px; font-style: normal; font-weight: normal; text-decoration: none; margin-top: 0px; margin-bottom: 0px;">
						<span style="padding-top: 0px; padding-bottom: 0px; margin-top: 0px; margin-bottom: 0px;">
							<span class="ddspellcheck" id="scayt3">jkhgkjh</span> 
						</span>
					</span>
					<br /> <!-- This doesn't get reoved -->
					&nbsp;
				</div>
				<div style="text-align: center;">
					<span style="padding-top: 0px; padding-bottom: 0px; margin-top: 0px; margin-bottom: 0px;">
						<span class="ddspellcheck" id="scayt2">gkjhgkjh</span> 
					</span>
				</div>

				<div>
					<span style="border: 0pt rgb(0, 0, 0); text-align: left; color: rgb(0, 0, 0); padding-top: 0px; padding-bottom: 0px; font-style: normal; font-weight: normal; text-decoration: none; margin-top: 0px; margin-bottom: 0px;">
						<span style="padding-top: 0px; padding-bottom: 0px; margin-top: 0px; margin-bottom: 0px;">
							<span class="ddspellcheck" id="scayt1">gkjhlgfkhj</span> 
						</span> 
					</span>
				</div>
			</div>
		</div>
	</div>
</div>

Other details (browser, OS, CKEditor version, installed plugins)

Problem can be reproduced from CKEditor 4.0 in IE10 and below

[Deepak Abburi]

Hi, I have a fix for the issue in core/dom/iterator.js file. I hope it help the engineer who works on the issue. if we replace the following code

if ( removePreviousBr ) {
				var previousSibling = block.getPrevious();
				if ( previousSibling && previousSibling.type == CKEDITOR.NODE_ELEMENT ) {
					if ( previousSibling.getName() == 'br' )
						previousSibling.remove();
					else if ( previousSibling.getLast() && previousSibling.getLast().$.nodeName.toLowerCase() == 'br' )
						previousSibling.getLast().remove();
				}
			}

			if ( removeLastBr ) {
			var lastChild = block.getLast();
	if ( lastChild && lastChild.type == CKEDITOR.NODE_ELEMENT && lastChild.getName() == 'br' ) {
					// Remove br filler on browser which do not need it.
if ( !CKEDITOR.env.needsBrFiller || lastChild.getPrevious( bookmarkGuard ) || lastChild.getNext( bookmarkGuard ) )
						lastChild.remove();
				}
			}

with the following

if ( removePreviousBr ) {
				var previousSibling = block.getPrevious();
				if ( previousSibling && previousSibling.type == CKEDITOR.NODE_ELEMENT ) {
				    if (previousSibling.getName() == 'br')
				        previousSibling.remove();

				    else {
				        var prevSib = previousSibling;
				        while (prevSib.$.childNodes.length) {
				            if (prevSib.getLast() && prevSib.getLast().$.nodeName.toLowerCase() == 'br') {
				                prevSib.getLast().remove();
				            }
				            prevSib = prevSib.getLast();

				        }
				    }
					  
			}
		}

			if ( removeLastBr ) {
			    var lastChild = block.getLast();
				if ( lastChild && lastChild.type == CKEDITOR.NODE_ELEMENT && lastChild.getName() == 'br' ) {
				    //Take care not to remove the block expanding <br> in non-IE browsers.
if ( CKEDITOR.env.ie || lastChild.getPrevious( bookmarkGuard ) || lastChild.getNext( bookmarkGuard ) )
				        lastChild.remove();
				}
	else if (lastChild.getLast() && lastChild.getLast().getName() == 'br')
				{
				    lastChild.getLast().remove();
				}
else if (lastChild.getLast().getLast() && lastChild.getLast().getLast().getName() == 'br') {
				    lastChild.getLast().getLast().remove();
				}
			}

it will fix the issue. But i am not sure about the other effects of that code change.

Thanks, Deepak

[Tade0]

Thank you for your contribution. I'm seeing that this works well as a solution to this specific edge case.

Chances are that the problem lies somewhere deeper, though, so we need to look for a more general solution - if one exists.

Investigating this now.

[Deepak Abburi]

Hi ,

what i see is if we have the things in the following format the issue happens.

<div>
    <span>
        text1<br>
        text2<br>
        text3<br>
     </span>
</div>

So the issue may be dealing with splitting the blocks where we have this type of pattern. When we split the things in this case we are not removing the old br in the markup. the extra line will be there depending on span with BR sometimes we may get extra line before where we justifying and some times we may get extra br after the line where we are justifying so it really depends on pattern. when we try iterator.getnextparagraph() the issue happens so may be when we split things we can go ahead remove the additional br at end of different child positions.On my debugging i have seen br in different child position depending on depending on spans inside div. so may be removal of br from text while we split block may fix the issue. But i am not sure on that.

I just want give this because may be this is a simple pattern which will help the situation more clear for you while you are trying to fix the things.

Thanks, Deepak

[Marek Lewandowski]

Unfortunately we won't be able to squeeze it into 4.6.0, retargeting for 4.6.1 though.

[Marek Lewandowski]

Moving to 4.6.2 minor release, as 4.6.1 is mostly about polishing 4.6.0.