Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#14590 closed Bug (fixed)

HtmlWriter adds extra line break after inline tag inserted

Reported by: David Pidcock Owned by:
Priority: Normal Milestone: CKEditor 4.5.10
Component: Core : Output Data Version:
Keywords: Cc:

Description

Steps to reproduce

  1. Start with Html as follows:
    <div>
      Some text goes here.
      <p>
         A paragraph.
      </p>
    </div>
    
  1. In Source, verify that there is a single line-break after the first line of text, before the <p> tag
  1. Switch to WYSIWYG, select the word "text" and hit the [B]old button, then switch back to Source mode.

Expected result

<div>
  Some <strong>text<strong> goes here.
  <p>
     A paragraph.
  </p>
</div>

Actual result

<div>
  Some <strong>text<strong> goes here.

  <p>
     A paragraph.
  </p>
</div>

Note that there is an extra line break between "here" and the <p> tag.

The extra line is added by htmlwriter.js @ line 120

if ( this._.afterCloser && rules && rules.needsSpace && this._.needsSpace )
        this._.output.push( '\n' );

This appears to be because this._.afterCloser doesn't distinguish between inline and block tags.

Impact is low for most purposes, but I have a project that integrates with CKeditor which uses <span> tags to mark positions (similar to bookmarks) in the document, and this extra line break is throwing off our dirty-detection (which strips out our spans before comparing)

Attachments (1)

main.html (806 bytes) - added by David Pidcock 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 3 years ago by David Pidcock

This appears to be simple to fix : In the last line of closeTag (line 220), change the line from

this._.afterCloser = 1;  

to

this._.afterCloser = CKEDITOR.dtd.$inline[tagName] ? 0 : 1;

comment:2 Changed 3 years ago by Jakub Ś

Resolution: invalid
Status: newclosed
Version: 4.5.9 (GitHub - master)

Please see http://docs.ckeditor.com/#!/guide/dev_output_format

I have set break indent rules for p, div and everything looks fine.

editor.on( 'instanceReady', function( evt ) {   
	this.dataProcessor.writer.setRules( 'p', {
		indent: true,
		breakBeforeOpen: true,
		breakAfterOpen: true,
		breakBeforeClose: true,
		breakAfterClose: true
	});
			
	this.dataProcessor.writer.setRules( 'div', {
		indent: true,
		breakBeforeOpen: false,
		breakAfterOpen: true,
		breakBeforeClose: false,
		breakAfterClose: true
	});
});	

comment:3 Changed 3 years ago by David Pidcock

Please retest and follow the steps exactly. If you don't switch to Source before bolding the text, everything looks fine.

Actually, that's another interesting result :

1) Start with the HTML as above.

2) Bold some text

3) Switch to Source (everything should look fine)

4) Toggle back to Wysiwyg and then Source again.

Result: Extra line break.

Changed 3 years ago by David Pidcock

Attachment: main.html added

comment:4 Changed 3 years ago by David Pidcock

The file I attached uses the code block you provided. I verified that it still experiences the issue as described.

comment:5 Changed 3 years ago by Jakub Ś

Resolution: invalid
Status: closedreopened

Thank for explaining it in more detail. I have actually missed that "switch to source and then source/wysiwyg again".

Yes, you are correct. Even with rules I have given, extra space is still inserted.

comment:6 Changed 3 years ago by Jakub Ś

Status: reopenedconfirmed

Problem can be reproduced at least from CKEditor 4.0 in all browsers.

@DavidPidcock if you wish this issue gets resolved fester, you can submit pull request - https://github.com/ckeditor/ckeditor-dev/pulls.

Here is explanation how it can be done: http://docs.ckeditor.com/#!/guide/dev_contributing_code.

Last edited 3 years ago by Jakub Ś (previous) (diff)

comment:7 Changed 3 years ago by David Pidcock

Thanks for revisiting :D

After reviewing the htmlwriter Rules code more closely, I realized my suggestion for a fix is too specific to my own scenario, and would not work in all situations.

comment:8 Changed 3 years ago by Tade0

Resolution: fixed
Status: confirmedclosed

Fixed with git:619cd60efb3b675508dd12eea9efb2b22fc77a46.

Thanks again, DavidPidcock!

comment:9 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.10
Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy