Unwanted linebreak on Firefox

[Artem]

If I use empty textarea replaced with CKEditor and post form I've '<br />' in Firefox 3.6. In IE8 and Chrome 4 I have empty string.

[wester]

Does this occur when Firebug is enabled or disabled?

See also ​http://dev.fckeditor.net/ticket/5253.

[Artem]

Replying to wester:

Does this occur when Firebug is enabled or disabled?

See also ​http://dev.fckeditor.net/ticket/5253.

This problem occurs even in safe mode. Ticket 5253 is not the same bug.

[Artem]

I checked previous version and confirm that this problem appears also in 3.1.1, but in 3.1 everything is fine.

[Artem]

To emulate this error you can use your samples (replacebyclass.html or replacebycode.html) with empty textarea by default. Open the page in the Firefox, scroll down to button and submit the form. You will see "<br />" as value of Posted Data.

[Artem]

This is th result of posting empty CKeditor value

[tarek]

This is happening in Firefox (3.6.3) on a mac platform as well without Firebug running:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 GTB7.0

CKeditor 3.2.1 is adding a

<br />

upon initialization. The tag is transmitted upon POST, when it should leave the input blank. It's making it especially difficult to validate database fields by checking to see if they are empty or not.

[tarek]

I've also confirmed that this is happening on the latest nightly build (as of 5/18/2010) on Mac FF 3.6.3 and PC FF 3.6.3. It does not occur on other browsers that I've tested. A patch would be most appreciated.

[Frederico Caldeira Knabben]

I confirm this has been introduced with version 3.1.1.

[Bruno Basto]

Replying to fredck:

I confirm this has been introduced with version 3.1.1.

is there a patch already?

[Bruno Basto]

hey guys, I guess I figured out whats the problem.

at _source\plugins\wysiwygarea\plugin.js

near line 397:

Restore the original document status by placing the cursor before a bogus br created (#5021). domDocument.createElement( 'br', { attributes: { '_moz_editor_bogus_node' : 'TRUE', '_moz_dirty' : "" } } ).replace( domDocument.getBody().getFirst() );

I'm not sure if we really need this:

domDocument.createElement( 'br', { attributes: { '_moz_editor_bogus_node' : 'TRUE', '_moz_dirty' : "" } } ).replace( domDocument.getBody().getFirst() );

does anyone know why we need this 'br'?

thanks!

[tarek]

I tried removing the lines above and at least when using the replace by class method of including a CKeditor, the problem remains. Might a change be necessary somewhere else when using the replace by class method?

[Simon Bartlett]

CKEditor/trunk/_source/core/dom/element.js, line 246 (version 3.1):

				this.append(
					CKEDITOR.env.opera ?
						this.getDocument().createText('') :
						this.getDocument().createElement( 'br' ) );

was changed (in version 3.1.1) to:

				var bogus = CKEDITOR.env.opera ?
						this.getDocument().createText('') :
						this.getDocument().createElement( 'br' );

				CKEDITOR.env.gecko && bogus.setAttribute( 'type', '_moz' );

				this.append( bogus );

reverting the code back fixes this issue.

[Simon Bartlett]

Please ignore/delete my last comment. I was mistaken.

[Simon Bartlett]

But reverting CKEditor/trunk/_source/core/htmlparser/fragment.js from 3.1.1 to 3.1 seems to fix the issue for me.

[Simon Bartlett]

I'm sorry for the multiple comments. But to be more specific, this bug was caused by changeset: [5103]

[Garry Yao]

Hi gentlemen, thanks for figuring this out, while we see it not a regression related to any of the above changes, as you can safely reproduce it on ​raw Mozilla editor, but it's surely something to fix (especially for those who rely on SQL to empty check emptiness).

[Garry Yao]

#4573 test could be reused here:
​run OR ​view source.

[Frederico Caldeira Knabben]

This one may also fix #5606.

[Alexander Clausen]

It seems that sometimes, the "bogus <br />" does not actually exist, so getBody().getLast() as used in the patch is null and causes an error on submitting the form.

If I understand the code correctly, the bogus node is created in contentDomReady in plugins/wysiwygarea/plugin.js, but only *if the body has no children*. But then, it tries to access the first child with domDocument.getBody().getFirst():

if ( CKEDITOR.env.gecko && !body.childNodes.length ) // <---
{
    setTimeout( function()
    {
        restoreDirty( editor );

        // Simulating keyboard character input by dispatching a keydown of white-space text.
        var keyEventSimulate = domDocument.$.createEvent( "KeyEvents" );
        keyEventSimulate.initKeyEvent( 'keypress', true, true, domWindow.$, false,
            false, false, false, 0, 32 );
        domDocument.$.dispatchEvent( keyEventSimulate );

        // Restore the original document status by placing the cursor before a bogus br created (#5021).
        domDocument.createElement( 'br', { attributes: { '_moz_editor_bogus_node' : 'TRUE', '_moz_dirty' : "" } } )
            .replace( domDocument.getBody().getFirst() ); // <-----
        var nativeRange = new CKEDITOR.dom.range( domDocument );
        nativeRange.setStartAt( new CKEDITOR.dom.element( body ) , CKEDITOR.POSITION_AFTER_START );
        nativeRange.select();
    }, 0 );
}

what am I missing here?

[Frederico Caldeira Knabben]

Note that a simulated keypress is sent to the browser, which creates the first node into <body>. Ad that point, it's replaced by the bogus <br>.

Those are some crazy hacks we need to live with to workaround browsers' bugs.

[Frederico Caldeira Knabben]

For reference, this is caused by [5389].

[Frederico Caldeira Knabben]

We should never change the DOM when retrieving the editor data.

[Frederico Caldeira Knabben]

The new patch is much similar to the first one, but doing that in the parsing phase, leaving the DOM untouched.

[Alexander Clausen]

ok, so the element gets created by a fake keypress. but isn't there a race condition between dispatching the keypress event and replacing the node? because i still get the following error in the error console (the one of firefox, not the firebug error console) on load:

"node is null" in "ckeditor/_source/core/dom/node.js". opening the firebug debugger (_not_ the console...), the stack trace looks like this:

insertBefore()
called by replace()
called by (?)() <--- this is the anonymous function passed to setTimeout() I posted above (ckeditor/_source/wysiwygarea/plugin.js line 423)

btw. I'm using iceweasel 3.5.11

the original error seems to be fixed by the new patch, though.

[Frederico Caldeira Knabben]

@sky1p, sorry, I can't understand the relation of your comment with this ticket.

[Sa'ar Zac Elias]

• The patch should be targeted only to FF.
• Say the textarea content is '<br />', it should not be removed.
• Going to source and back to WYSIWYG mode always removes the last br.

[Sa'ar Zac Elias]

#6030 is a DUP and as mentioned there the bug also affects Opera.

[Frederico Caldeira Knabben]

The major problem here is that it's quite hard for us to understand whether a <br> is a real thing or a bogus. So, as a safeguard, we make the editor in a way that it'll not handle inline content directly inside <body>, expect when enterMode is BR (which is a non recommended mode).

So, right now, the most urgent thing is having the unwanted <br> disappear, which will impact on 99% of our users. Then, we can handle the "wanted" <br> case separately, if our community shows this is necessary.

The good thing about the r- is that I was able to rethink about the problem, so I'm proposing a solution that's really touching the parsing phase, so it will work with other data processors also.

[Frederico Caldeira Knabben]

Fixed with [5750].

[tarek]

Excellent news. Your work is very much appreciated.

[lehmanjas]

Replying to fredck:

Fixed with [5750].

I added the patch described in 5750 but noticed that if the textarea never recieves focus before someone submits the form then the <br /> tag doesn't get removed and you end up having the same problem.

[Frederico Caldeira Knabben]

Replying to lehmanjas:

Replying to fredck:

Fixed with [5750].

I added the patch described in 5750 but noticed that if the textarea never recieves focus before someone submits the form then the <br /> tag doesn't get removed and you end up having the same problem.

Sounds strange. The processing happens in any case, no matter the editor focus.

Can you provide us a test page for it?

[lehmanjas]

Replying to fredck:

Replying to lehmanjas:

Replying to fredck:

Fixed with [5750].

I added the patch described in 5750 but noticed that if the textarea never recieves focus before someone submits the form then the <br /> tag doesn't get removed and you end up having the same problem.

Sounds strange. The processing happens in any case, no matter the editor focus.

Can you provide us a test page for it?

Here is the test link: ​http://coedu.rc.usf.edu/research_staff/test/testckeditor.php. It is set up to do an alert when you press the test button and it shows the contents of the box.

I am using Firefox 3.6.8 and if I just press the test link without selecting inside the box then I get an alert with the <br /> tag. Then if I select inside the box I get an empty alert. Then after that I can hit the test button and get an empty alert.

[Frederico Caldeira Knabben]

Test case for comment:34 using the JQuery adapter

[Frederico Caldeira Knabben]

Test case for comment:34 using JavaScript and DOM

[Frederico Caldeira Knabben]

Replying to lehmanjas:

I've just attached two reduced TCs for your page. It's working well for me with the current trunk, so it looks like there is something else missing in your code.

Better to wait for the 3.4 to be released, which is supposed to happen soon.

[Slavon]

​http://smartmiltoys.com/kidkraft-dollhouse/