search

sproutcore / rich-text-editor

A rich text editor for SproutCore.
Other
6 stars 6 forks source link

Issues with forceLineBreaks mode #16

Open nicolasbadia opened 10 years ago

nicolasbadia commented 10 years ago

To reproduce the issue, set forceLineBreaks to true and add this HTML: 

<p>some text</p><br>

If you put the cursor at the end of the paragraph, enter must be press twice to add a line break.

dcporter commented 10 years ago

This sort of thing always needs to include which browser. That's gotta be chrome, right?

Did you manage to enter that HTML from the keyboard? If so, what were the keystrokes? With forceLineBreaks, there should be no <p> elements generated.

If not, then we need to figure out whether it's reasonable for us to try to handle any external input perfectly. I'm tempted to declare that input has to be valid p-wrapped or br-delimited ahead of time or we're not responsible for it.

nicolasbadia commented 10 years ago

Yep chrome. I implemented https://github.com/ajaxorg/ace with the editor so I can see and edit the source code. That's how I add HTML. But I found the issue with existing HTML wrote by clients.

dcporter commented 10 years ago

My point being, I'm pretty sure I've got something that can behave well with HTML that it generates from the current version of itself. Dealing with browser-specific issues is such a pain that e.g. GitHub doesn't use contenteditable. So we have to draw the line somewhere I think. The question is where.

nicolasbadia commented 10 years ago

Other bad issue related to forceLineBreaks being set to true. Type some text, set it as H1, press return to make a new line (you will have to press it twice), type some text again, try to set it as paragraph (it will not work). Here is the HTML you get:

<h1>aaaa<br><p>bbbb</p></h1><br>
dcporter commented 10 years ago

Okay that I haven't played with. I'll give it a look ASAP. (By the way I'm likely to be out of touch for a few days; apologies for the delay!)

dcporter commented 10 years ago

So here's where we're at with these.

The editor does a good job with input that it has generated – carefully – but it's not equipped to deal with arbitrary markup coming in from externally, including common markup from earlier versions of WYSIWYG (e.g. the <p>Hi.</p><br> example). In order to handle that, we'd have to do much more extensive (and expensive, and probably fragile) DOM editing than we currently do in order to change any arbitrary markup into markup that it can handle – and it would need to be able to reliably convert from <br> style line-breaks to<p>-style in case forceLineBreaks changes. We need to decide if this is a design goal, or if it's up to the developer to sanitize their input markup for use with forceLineBreaks.

As you found, forceLineBreaks mode is currently super buggy when used with the block-level formatters (e.g. <h1>). In line-breaks mode we essentially need to intercept "formatBlock" execCommand calls and manually apply them to the outermost block rather than the innermost <br>-flanked section. We also need to handle formatBlock "p" calls by removing the block (and re-appending a <br> I believe).

Line-breaks mode also doesn't currently support the text-align buttons very well. Text-align requires a block-level element, and line-breaks mode is explicitly trying to avoid block-level elements. It's about the same level of challenge as the header elements; we need to make sure that we're making changes to the correct element, and certain commands (e.g. return-to-default-alignment) need to result in removing the block-level element rather than changing it.

By the way, any block-level element in line-breaks mode is going to run into the hit-enter-twice issue.

Given all this, I'm going to mark forceLineBreaks mode as experimental for now, and work on these issues as we go. Any bugs with non-linebreaks mode should take higher priority and should get their own Issues.