Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#13194 closed Bug (invalid)

CKEditor adds empty paragraph to the content on startup (inline mode)

Reported by: ctrl Owned by:
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:

Description

CKEditor adds an empty paragraph on startup.

Steps to reproduce:

  1. Attach CKEditor to an element on the page
  2. Load the page in Chrome/Firefox (I think it will break in all browsers however)

Result: a new paragraph has been added to the content Expected: the content to be empty (untouched by CKEditor)

This breaks an plugin I created - this basically behaves as an HTML5 placeholder. However, it stopped to work when I upgraded to the last version of CKEditor.

See the attached page where you can see the code and the screencast.

Hint - I think this is caused by #12630

Attachments (2)

screencast.gif (1.6 MB) - added by ctrl 9 years ago.
screencast
test.html (563 bytes) - added by ctrl 9 years ago.
demo of the issue

Download all attachments as: .zip

Change History (16)

Changed 9 years ago by ctrl

Attachment: screencast.gif added

screencast

Changed 9 years ago by ctrl

Attachment: test.html added

demo of the issue

comment:1 Changed 9 years ago by Jakub Ś

Resolution: invalid
Status: newclosed
Version: 4.4.7

This is correct behaviour of CKEditor - http://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-enterMode.

By default editor uses enter mode set to P. You can change it to BR in order not to see the paragraph, I would however recommend changing custom plugin behaviour.

comment:2 Changed 9 years ago by ctrl

Version: 4.4.7

No, this is NOT the correct behaviour! Did you read what I wrote, watched the screencast or run the page I attached?!

This is a REGRESSION, it wasn't like this till one or two versions of CKEditor ago. Moreover, "enterMode" has nothing to do here - I'm not pressing Enter. I'm not doing anything - CKEditor mangles the content ON PAGE LOAD - before I touch the page.

comment:3 Changed 9 years ago by Piotrek Koszuliński

Version: 4.4.7

I watched the screencast and I confirm - this is the correct behaviour. Empty editor must be filled with a paragraph with a <br>. Inline editors based on elements like <p> or <h1> which cannot contain another blocks are filled with <br> only. This is how it works on your screencast and this is how it should work. Otherwise, editor may invisible to the user who won't be able to type in it. If it worked differently, it was a bug and the bug was fixed.

This breaks an plugin I created - this basically behaves as an HTML5 placeholder. However, it stopped to work when I upgraded to the last version of CKEditor.

Unfortunately, your plugin might have been based on an incorrect behaviour of CKEditor. This means that you will need to find another way to implement placeholder.

comment:4 Changed 9 years ago by ctrl

For the record, what you mean with "Otherwise, editor may invisible to the user who won't be able to type in it."?

comment:5 Changed 9 years ago by Piotrek Koszuliński

That an empty block (with zero content) has height 0px unless it has some padding or min-height. That makes it uneditable.

We could however set the min-height style to some value. It would be a poor solution, because I don't think that we can easily check what height it should have. But most importantly - we need to create that paragraph at some point anyway. It is crucial for good editing experience. So when could we do it? On focus? That's a poor solution again, because it has to be integrated with the undo manager, it has to work correctly with focus and it still may cause editor's height change. What's more - we used that solution and it caused issues (#12630), because selection was placed outside that paragraph and we couldn't force it inside.

As you can see - every other solution is tricky and far from perfect. Therefore in #12630 we changed this behaviour to always have a filler (wrapped with a paragraph if config.autoParagraph is true and it always should be true) inside the editable.

comment:6 Changed 9 years ago by ctrl

I understand your arguments. However, even with this change, the UX is far from good - if there is nothing in the contenteditable, the user will still have absolutely no indication that there is an editable part in the page. That is the reason why people set min-height and also add a placeholder on their pages - to indicate people that they can write something on this place.

However, with the introduced change now I cannot imagine how can we create a proper placeholder, since we cannot rely on the empty content anymore. :(

comment:7 Changed 9 years ago by Piotrek Koszuliński

You could never rely on the empty content. User could focus the editor, remove all data and blur it. There would be an filled block after that but the placeholder should be displayed anyway.

As for the other solutions, I think that if the :empty pseudoclass does not work (you could add placeholder by ::before) and I know that browsers implemented that pseudoclass poorly, you will need to use JS. I would base it on the editor#change event and adding class to the editable element, so a placeholder can be added in the first block using ::before.

comment:8 Changed 9 years ago by Piotrek Koszuliński

PS. For performance reasons I advise not to use editor.getData() on every editor#change, but a direct access to editor.editable().getHtml(). Debouncing editor#change would be nice too.

comment:9 in reply to:  7 Changed 9 years ago by ctrl

Replying to Reinmar:

You could never rely on the empty content. User could focus the editor, remove all data and blur it. There would be an filled block after that but the placeholder should be displayed anyway.

This wasn't the scenario until recently. When you remove all data and blur, the content would be empty, without any <p><br></p> or only <br>. That's why we had elegant solution based on :empty pseudo class.

Replying to Reinmar:

As for the other solutions, I think that if the :empty pseudoclass does not work (you could add placeholder by ::before) and I know that browsers implemented that pseudoclass poorly, you will need to use JS. I would base it on the editor#change event and adding class to the editable element, so a placeholder can be added in the first block using ::before.

I'm trying to imagine this approach: We set the placeholder initially using JavaScript and ::before selector (and only if we don't have content already). This is already bad approach, because the user will see blank field and as soon as JavaScript initializes, the placeholder will appear. But let's continue: Then, we subscribe to editor#change event and as soon as we receive it, we remove the placeholder, if the content is empty. The question is - how do we check if the content is empty? And what exactly means empty content? For CKEditor it seems empty content is content, which contains <p><br></p>, on only <br> (in case of H1 ), or who knows what else. This means on editor#change event we will have to compare the current content with the content, which CKEditor sets in case of "empty" editor, or what? I'm sorry, but all this just does not make sense?

Last edited 9 years ago by ctrl (previous) (diff)

comment:10 Changed 9 years ago by Piotrek Koszuliński

This wasn't the scenario until recently. When you remove all data and blur, the content would be empty. That's why we had elegant solution based on :empty pseudo class.

I'm afraid you didn't test this on enough browsers or you didn't thought about checking some scenarios. The first case I checked on Chrome (ctrl+a, backspace) gives me:

<h1><small><a data-cke-saved-href="http://en.wikipedia.org/wiki/Apollo_11" href="http://en.wikipedia.org/wiki/Apollo_11"></a></small>​</h1>

Which is actually a totally broken result, because there should be no link or small elements (it's already fixed in 4.5).

Another case - I cleared the content, wrote few letters, pressed the same combination again and I have <p><br></p> just like now.

I don't deny that some browsers in some cases may remove all content. But it does not mean that we have to adapt this problematic approach.

But how do you check if the content is empty or not? And what exactly means empty content? For CKEditor it seems empty content is content, which contains <p><br></p>, on only <br> (in case of H1 ), or who knows what else. I'm sorry, but this just does not make any sense?

You're right - it's not that trivial. There are many cases to consider - ctrl+a + backspace/delete, new page, deleting content letter by letter, selecting all with mouse, etc. That's why implementing an editor is that complex. But the fact that something is not trivial does not make it unreasonable. I can't launch a rocket, but I don' think that it's someone's fault. Life isn't easy.

As for this, a simple regexp could handle most cases because statistically most often you will get (depending on editor.blockless):

  • false: <block>(<br />|\u00a0)?</block>
  • true: (<br />|\u00a0)?

NBSP may be used instead of <br> on older IEs, hence the alternative.

Although, there may be some leftovers like attributes on the block or empty inline elements. Attrbiutes are easy to accept in the regexp, empty elements... perhaps too.

But CKEditor of course has tools for operations like this, although, I would need to research what may appear in the editor and find the right settings. In general, we mainly use the CKEDITOR.dom.walker for scanning DOM.

comment:11 Changed 9 years ago by ctrl

Thanks for the above research. There is something I mentioned above, but will repeat it with more details - to create an placeholder via JavaScript is not the right way. Why?

Because CKEditor loads asynchronously. This means while it loads, then until we check the content, and decide shall we add a placeholder or not, the user will see nothing, but blank space and then suddenly - a placeholder.

This is bad from UX point of view and I don't know for you, but for me is unacceptable. Hope you understand.

comment:12 Changed 9 years ago by Piotrek Koszuliński

I'm not sure if you mean a textarea (or other element that user sees before an editor creator method is called) or the moment when the editor already started but is not ready yet. The former is easy to fix in exactly the same way or with a CSS based solution. The latter is indeed tricky, but it wouldn't work before either because we are talking about an editable which does not exist yet (in case of framed editor). One more thing - I don't think that a placeholder should be visible before the editor is ready. Of course it is a very short moment, but by definition you don't want users typing in an app which isn't ready.

comment:13 Changed 9 years ago by ctrl

No, Reinmar, I'm not talking at all about framed CKEditor. Of course I meant inline editing mode. Sorry if that wasn't clear. See this screenshot(http://pasteboard.co/2Ma0SHw8.png)? - there are three editors (Title, Subtitle and Contet) with placeholders. Isn't that pretty? With the change you made we cannot achieve this anymore, that's why I'm screaming and kicking.

Btw, I saw that on load, CKEditor removes the whole content and puts it back. Is that really needed in case of inline editing?

comment:14 Changed 9 years ago by Piotrek Koszuliński

With the change you made we cannot achieve this anymore, that's why I'm screaming and kicking.

I understand. But as we agreed already - it just needs to be implemented differently :)

Btw, I saw that on load, CKEditor removes the whole content and puts it back. Is that really needed in case of inline editing?

Yes, this happens because editor first uses an element on which it was initialised with exactly the HTML that is inside. But that HTML is just a "data" that needs to be loaded (it means parsing, filtering, preparing for editing, etc.). So at some point the initial HTML which is only a source of data is replaced with a processed data. In case of a simple editor it might not be visible but if you had some widgets or things like iframes, empty anchors, etc. then you would notice that they change.

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