Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with forceLineBreaks mode #16

Open
nicolasbadia opened this issue Jun 5, 2014 · 6 comments
Open

Issues with forceLineBreaks mode #16

nicolasbadia opened this issue Jun 5, 2014 · 6 comments

Comments

@nicolasbadia
Copy link
Member

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
Copy link
Member

dcporter commented Jun 5, 2014

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
Copy link
Member Author

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
Copy link
Member

dcporter commented Jun 5, 2014

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
Copy link
Member Author

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
Copy link
Member

dcporter commented Jun 5, 2014

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 dcporter changed the title Sometime needs to press twice the enter key to add a line break General behavior discussions Jun 10, 2014
@dcporter dcporter changed the title General behavior discussions Issues with forceLineBreaks mode Jun 10, 2014
@dcporter
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants