#5293 closed Bug (fixed)
Unwanted linebreak on Firefox
Reported by: | Artem | Owned by: | Frederico Caldeira Knabben |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 3.4 |
Component: | Core : Output Data | Version: | 3.1.1 |
Keywords: | Confirmed Firefox Opera | Cc: | tarek@…, b.basto@…, alex@… |
Description
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.
Attachments (7)
Change History (42)
comment:1 follow-ups: 2 3 Changed 15 years ago by
comment:2 Changed 15 years ago by
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.
comment:3 Changed 15 years ago by
I checked previous version and confirm that this problem appears also in 3.1.1, but in 3.1 everything is fine.
comment:4 Changed 15 years ago by
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.
Changed 15 years ago by
Attachment: | ckedi2010-04-27_132117.jpg added |
---|
This is th result of posting empty CKeditor value
comment:5 Changed 15 years ago by
Cc: | tarek@… added |
---|
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.
comment:6 Changed 15 years ago by
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.
comment:7 follow-up: 8 Changed 15 years ago by
Component: | General → Core : Output Data |
---|---|
Keywords: | Confirmed added |
Milestone: | → CKEditor 3.4 |
Version: | 3.2 → 3.1.1 |
I confirm this has been introduced with version 3.1.1.
comment:8 Changed 15 years ago by
Cc: | b.basto@… added |
---|
Replying to fredck:
I confirm this has been introduced with version 3.1.1.
is there a patch already?
comment:9 Changed 15 years ago by
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!
comment:10 Changed 15 years ago by
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?
comment:11 follow-up: 12 Changed 15 years ago by
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.
comment:13 Changed 15 years ago by
But reverting CKEditor/trunk/_source/core/htmlparser/fragment.js from 3.1.1 to 3.1 seems to fix the issue for me.
comment:14 Changed 15 years ago by
Milestone: | CKEditor 3.4 → CKEditor 3.3 |
---|
I'm sorry for the multiple comments. But to be more specific, this bug was caused by changeset: [5103]
comment:15 Changed 15 years ago by
Milestone: | CKEditor 3.3 → CKEditor 3.4 |
---|
Changed 15 years ago by
Attachment: | 5293.patch added |
---|
comment:16 Changed 15 years ago by
Keywords: | Review? added |
---|---|
Owner: | set to Garry Yao |
Status: | new → assigned |
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).
comment:19 Changed 14 years ago by
Cc: | alex@… added |
---|
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?
comment:20 Changed 14 years ago by
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.
comment:22 Changed 14 years ago by
Status: | review → review_failed |
---|
We should never change the DOM when retrieving the editor data.
Changed 14 years ago by
Attachment: | 5293_2.patch added |
---|
comment:23 Changed 14 years ago by
Owner: | changed from Garry Yao to Frederico Caldeira Knabben |
---|---|
Status: | review_failed → review |
The new patch is much similar to the first one, but doing that in the parsing phase, leaving the DOM untouched.
comment:24 Changed 14 years ago by
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.
comment:25 Changed 14 years ago by
@sky1p, sorry, I can't understand the relation of your comment with this ticket.
comment:26 Changed 14 years ago by
Status: | review → review_failed |
---|
- 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.
comment:27 Changed 14 years ago by
Keywords: | FireFox Opera added |
---|
#6030 is a DUP and as mentioned there the bug also affects Opera.
Changed 14 years ago by
Attachment: | 5293_3.patch added |
---|
comment:28 Changed 14 years ago by
Keywords: | Firefox added; FireFox removed |
---|---|
Status: | review_failed → review |
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.
comment:29 Changed 14 years ago by
Status: | review → review_passed |
---|
comment:30 follow-up: 32 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed with [5750].
comment:32 follow-up: 33 Changed 14 years ago by
comment:33 follow-up: 34 Changed 14 years ago by
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?
comment:34 follow-up: 35 Changed 14 years ago by
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.
Changed 14 years ago by
Attachment: | 5293_Comment34_TC_JQuery.html added |
---|
Test case for comment:34 using the JQuery adapter
Changed 14 years ago by
Attachment: | 5293_Comment34_TC_JS.html added |
---|
Test case for comment:34 using JavaScript and DOM
comment:35 Changed 14 years ago by
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.
Changed 9 years ago by
Attachment: | ffmpeg-autumn-logo_muxson.2.txt added |
---|
Does this occur when Firebug is enabled or disabled?
See also http://dev.fckeditor.net/ticket/5253.