Opened 18 years ago

Closed 16 years ago

#205 closed Bug (fixed)

IE: <br> removed from the end of paragraphs

Reported by: shill@… Owned by: Martin Kou
Priority: Normal Milestone: FCKeditor 2.5
Component: General Version: FCKeditor 2.6
Keywords: Confirmed HasPatch SD-COE Cc: pkdille, Jean-Marc Libs, Nyloth

Description

If you have the following code in the WYSIWYG

<p>line one</p> <p>line two</p>

If at the end of line one you try and insert a <br> it gets removed when you view the source. However if you insert it at the start of line two then it is fine.

This has been replicated in IE 6 & 7 Works fine in Firefox

Attachments (2)

br.html (315 bytes) - added by Frederico Caldeira Knabben 18 years ago.
Test page to show the final results
patched_editorcode.tgz (76.8 KB) - added by sdenovan 17 years ago.

Download all attachments as: .zip

Change History (30)

comment:1 Changed 18 years ago by Frederico Caldeira Knabben

Keywords: Discussion Pending added
Version: FCKeditor 2.4

I would like to open a discussion regarding this issue.

It seams that the <br /> at the end of a paragraph has no value, neither semantic nor stylistic. Possibly I'm wrong. I've attached a sample page that shows the final results.

The thing that pushed us on making this cleanup is that Firefox loves to leave extra BRs in the code and we had complains regarding it.

So the first question is: what are the motivations to have the <br /> at the end of paragraphs?

Changed 18 years ago by Frederico Caldeira Knabben

Attachment: br.html added

Test page to show the final results

comment:2 Changed 18 years ago by aburgel

i have a very similar issue.

if you have code that looks like this:

<p><strong>A</strong><em>B</em></p>

if you try to insert a <br> between A and B, the BR will disappear in IE.

comment:3 Changed 18 years ago by aburgel

after looking further into this, i think this is definitely a bug.

i think this is caused by FCKXHtml._AppendNode which removes any <BR> if it is the last child.

that might make sense if the parent is a block-level element, but not for any other parent element. and the code in _AppendNode doesn't make that distinction.

i think this should probably be rolled back, because even if you can tell whether the parent element is block level, you don't know what styles might be applied to it which could change the behavior.

comment:4 in reply to:  3 Changed 18 years ago by Frederico Caldeira Knabben

Summary: <br> removed from IEIE: <br> removed from the end of paragraphs

Replying to aburgel:

after looking further into this, i think this is definitely a bug.

The problem you have described has already been solved. Please try our nightly build. For more info, click here.

This ticket is related to <br> tags at the end of paragraphs.

comment:5 in reply to:  description ; Changed 18 years ago by Anders Bjerkholt

I suppose this bug has been solved. I'll check the nightly build, but my problem was not exactly as mentioned: When inserting a linebreak within an paragraph i fckeditor 2.4 with IE7. The <br /> is actually shown when switching to source code, but removed when saving. The <br /> in middle of the code below was removed. But, when using Shift+Enter twice to get 2 linebreaks one is removed and one is kept.

<td> <p align="center"> <font size="2"> <img src="clipart/soklurt.jpg" alt="" /> </font>

<br />

<a href="index.php?art=82"> <font size="2">Click me</font> </a> </p> </td>

comment:6 in reply to:  5 Changed 18 years ago by Frederico Caldeira Knabben

Replying to abjerkho:

When inserting a linebreak within an paragraph i fckeditor 2.4 with IE7.

Please try the nightly first of all.

The <br /> is actually shown when switching to source code, but removed when saving.

It sounds really strange because the procedure that generates the output for post is the same as the one for the source view. Please check it again.

If you really think you have a problem, please open a dedicated ticket for it, because this side discussion is polluting this ticket.

comment:7 Changed 18 years ago by Frederico Caldeira Knabben

#285 is a DUP of this one.

comment:8 Changed 18 years ago by Michael Bu

Similar problem still exists with latest build and with Firefox as well. I opened a ticket for it but was closed as DUP of this one. Please refer to #285 for repo steps.

In my opinion, the fact that it behaves differently in normal mode and source mode is REALLY confusing. If you were to insist on this behavior (silently remove line breaks), you shouldn't allow user to enter them in normal mode in first place.

The main reason for these line breaks to stay is sometimes you want to enter line breaks and sometimes you want paragraphs. But an average user has no idea about the differece of these two and not to mention the difference of "Enter" vs "Shift-Enter". The safer way to do was to enforce the "br" mode on both EnterMode and ShiftEnterMode settings so user would get a "consistent" behavior they expacted.

Therefore, empty line breaks at the end of a paragraph simply represents the result of an user's act and that's exactly what they wanted, doesn't matter if that's useful to us or anybody else.

comment:9 Changed 18 years ago by Frederico Caldeira Knabben

Keywords: Pending removed
Milestone: FCKeditor 2.6

comment:10 in reply to:  5 Changed 18 years ago by Johnny Cage

Hi. I've tested the nightly build but I'm still having the problem. The <br /> tags inserted through SHIFT + ENTER disappear when switching from Source perspective to preview. I'm not experienced with the editor but I've been reading the tickets. Thanks.

Replying to abjerkho:

I suppose this bug has been solved. I'll check the nightly build, but my problem was not exactly as mentioned: When inserting a linebreak within an paragraph i fckeditor 2.4 with IE7. The <br /> is actually shown when switching to source code, but removed when saving. The <br /> in middle of the code below was removed. But, when using Shift+Enter twice to get 2 linebreaks one is removed and one is kept.

<td> <p align="center"> <font size="2"> <img src="clipart/soklurt.jpg" alt="" /> </font>

<br />

<a href="index.php?art=82"> <font size="2">Click me</font> </a> </p> </td>

comment:11 Changed 17 years ago by Frederico Caldeira Knabben

Cc: Pascal.KUSTNER@… added
Keywords: SD-COE added

comment:12 Changed 17 years ago by Frederico Caldeira Knabben

Cc: jean-marc.libs@… patrice.weber@… added

comment:13 Changed 17 years ago by Frederico Caldeira Knabben

Cc: pkdille Jean-Marc Libs Nyloth added; Pascal.KUSTNER@… jean-marc.libs@… patrice.weber@… removed

comment:14 in reply to:  1 Changed 17 years ago by craigr

We get this issue in the following situation;

<p><strong>my heading</strong> Next line of text</p>

In the WYSIWYG mode when you try and insert a break after the "g" in heading it will not insert unless you hit shift + enter twice.

When I look at the code when it works it looks like;

<p><strong>my heading<br /></strong> Next line of text</p>

The issue is only in IE and not Firefox.

The issue also occurs in the middle of a <p> </p> tag.

Replying to fredck:

I would like to open a discussion regarding this issue.

It seams that the <br /> at the end of a paragraph has no value, neither semantic nor stylistic. Possibly I'm wrong. I've attached a sample page that shows the final results.

The thing that pushed us on making this cleanup is that Firefox loves to leave extra BRs in the code and we had complains regarding it.

So the first question is: what are the motivations to have the <br /> at the end of paragraphs?

comment:15 Changed 17 years ago by sdenovan

There appear to be two issues:

Issue 1

<br /> Cannot be arbitrarily trimmed. The editor will properly accept and render the <br /> elements but the backend xhtml building code currently drops them breaking wysiwyg.

Example:

<click numbered list icon>
one<shift-enter>
<shift-enter>
<enter>
two

The editor correctly displays this as:

1. One


2. Two

Click Source twice. Now the list becomes:

1. One
2. Two

I have commented out the trim statement in the .tgz attached files and have not experienced any adverse effects under IE6, IE7 and Firefox (2.0.0.5 Linux). Your mileage may vary...

Issue 2

IE and Firefox both ignore a single <br /> if it is immediately followed by a closing block level tag. This means that if you want a single space, you actually need two breaks. However, the editor will correctly display two <br /> elements as two new lines. Clicking Source twice shows that one will be dropped based on the generated code. This isn't really an fckeditor issue. Its probably specified in a W3C specification somewhere. However, it breaks wysiwyg in the editor so its something that should be looked at. Its important to note that if anything follows the last <br />, the browser will display the newlines properly.

In the attached .tgz I have patched the relevant files with a workaround to this problem. Roughly:

If you are in a block level type tag (not inline) and the last element in the block is <br /> add a single &nbsp; before the block level tag is closed. This ties into existing logic that places &nbsp; into blank blocks and therefore requires that the following be set (which it is by default) in fckconfig.js:

FCKConfig.FillEmptyBlocks = true ;

Note: The .tgz is against fckeditor 2.4.3. It is not fully compressed and is designed as a proof of concept and discussion point. Simply replace files with the same name in your install to evaluate the changes.

_source details

You can ignore this unless you're planning on patching and recompressing the source file.

I'm not sure how fckxhtml.js (the uncompressed source) gets translated into those two files but line 134 and following in fckxhtml.js (version 2.4.3) were changed to:

	// Trim block elements. This is also needed to avoid Firefox leaving extra
	// BRs at the end of them.
//	if ( isBlockElement )
//		FCKDomTools.TrimNode( xmlNode, true ) ;

  if (xmlNode.childNodes.length == 0 ||
      (!FCKListsLib.InlineChildReqElements[xmlNode.nodeName] &&
       xmlNode.childNodes[xmlNode.childNodes.length-1].nodeName=='br'))
	{
                // Unchanged below here...
		if ( isBlockElement && FCKConfig.FillEmptyBlocks )
		{
			this._AppendEntity( xmlNode, this._NbspEntity ) ;
			return xmlNode ;
		}
...

fckeditorcode_ie.js was uncompressed in the relevant area so if I have a typo above, check this file for the exact changes (starting on line 94).

Changed 17 years ago by sdenovan

Attachment: patched_editorcode.tgz added

comment:16 Changed 17 years ago by Frederico Caldeira Knabben

Keywords: Confirmed HasPatch added; Discussion removed
Milestone: FCKeditor 2.6FCKeditor 2.5

The solution proposed by sdenovan is definitely the way to go.

We must just be sure we will not have issues with the bogus BRs, which are used extensively in our code. Theoretically it should work out of the box, but lots of tests must be done.

comment:17 Changed 17 years ago by Martin Kou

Owner: set to Martin Kou
Status: newassigned

comment:18 Changed 17 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [640].

Click here for more info about our SVN system.

comment:19 Changed 17 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: closedreopened

While checking #1093 over Firefox, I've noted that the enter key was completely broken in that case. A JavaScript error was thrown and the editor was loosing data.

Tracking down the problem, I've found out that that bug was caused by [640]. Which forced me to review our approach here.

It seams that the correct way is: if a paragraph ends with a "real" <br>, a &nbsp; must be added to it, so the empty line space visible in the editor is rendered in the final output.

Now, the question: What's a "real <br>"?

In non IE browsers, we use the so called "bogus BR" to "expand" empty blocks and lines so they get visible while editing. The bogus BR will "always" be the last one in a paragraph (block). As <br> can be used to expand those empty spaces, we could also think about leaving the bogus BRs, so the expansion would be done in the final output too. It works perfectly, but the we have IE...

IE instead, automate things a little bit, in a good way. It understand that, in editing mode, a user wants to fill empty spaces, so it automatically expands them, making room there for typing. It is valid for empty blocks, like <p></p>, but also for <br>, giving space after it (in a new line) to type text. So, we don't use bogus BRs here.

If we would opt to leave the bogus BRs produced in Firefox, when opening the same contents in IE, those BRs would get expanded, adding an undesired line space after them.

So, the final solution is quite simple:

  1. If not IE, simply remove the last BR from blocks. This is done in FCKDomTools.TrimNode.
  2. Then, if the block still ends with a BR (in all browser now), we add a &nbsp; after it, if FillEmptyBlocks is enabled.

And that's it. It seams that no further things are needed here. It is actually quite a simple situation.

The conclusion: I've reverted [640] and introduced the above solution with [720].

Martin, please take a look at the changes, and close this ticket if you feel that everything is working well.

comment:20 Changed 17 years ago by Martin Kou

There's still a bug with this approach. Say, you open sample01.html, and press Ctrl-Enter at the end of the paragraph once (no line break is generated here). Press the Source button twice, and now you get one extra line break. Same for two or more Ctrl-Enter presses - there's always one extra linebreak after switching to source mode and back.

comment:21 Changed 17 years ago by Martin Kou

The test case in my last comment applies to IE only.

comment:22 Changed 17 years ago by Martin Kou

I've improved upon [720] by removing the special case for IE altogether, in [731]. The rationale is that, for all browsers that we support or are going to support (IE, FF, Opera, Safari), a single <br> at the end of a block node is always not rendered. So, the strategy of deleting the final <br> (because it is invisible) and putting a placeholder &nbsp; (because it forces the previous <br> to be rendered) in its place should be valid for all browsers, even for IE.

comment:23 Changed 17 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: reopenedclosed

Martin, the problem you have faced three comments above has nothing to do with [720]. It was a side effect of [725] instead.

Regarding [731], I've reverted it with [732]. It has introduced a lot of issues with <br> handling it IE. Let me try to explain it.

As I've described before, IE automatically expands <br> tags so you can insert text right after it. It doesn't happens always, for example when making DOM inclusions of BRs, like the enter key handler does. To solve the problem, we have found some strategies to use the selection, expanding it "manually".

With [731], you have assumed that bogus BRs are the way to go with IE, like in other browser, but we must strongly avoid them because of the "magic" expansion thing. After your changes, it was quite easy to find situations when a bogus BR get expanded by IE, introducing an unexpected line space.

As I've said, the problem was on [725], and I've fixed it with [733].

I'm closing this ticket now, but feel free to reopen it if you thing there is still some buggy situation here. But this time, let's talk before.

comment:24 Changed 17 years ago by Nyloth

Resolution: fixed
Status: closedreopened

Sorry, but this bug is not fixed from my point of view.

Simply try to do the example from Issue 1 describe above by sdenovan (08/01/07 02:03:15) with Firefox2 on the nightly build. You will see that going to the source and then back to the wysiwyg will remove the last BR. I'm not discussing weither the last BR should be shown or not is not. It should simply be coherent and the user must see the same display all the time (when writing, when coming back from the source, etc.)

This is why I have to reopen this bug.

comment:25 Changed 17 years ago by Martin Kou

Milestone: FCKeditor 2.5 BetaFCKeditor 2.5
Version: FCKeditor 2.4FCKeditor 2.5 Beta

comment:26 Changed 17 years ago by Martin Kou

Resolution: fixed
Status: reopenedclosed

Fixed with [1085] and [1086].

The bug was caused by duplicated bogus BR removal logic in fckxhtml.js which removes both the invisible BR tag at the end of a block element as well as a visible BR tag just before it. This causes a come back of the old problem.

comment:27 Changed 16 years ago by dreamage

Resolution: fixed
Status: closedreopened
Version: FCKeditor 2.5 BetaFCKeditor 2.6

I tested this with the latest 2.6.4 and it seems there is a problem with Firefox. If you paste this in the source view

<div> <h5>aaa</h5> <br class="clear"/> </div>

then switch back to design view and then back to source view, the BR is gone in FF3 but still there in IE7.

comment:28 Changed 16 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: reopenedclosed

We must have a dedicated it for this.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy