Ticket #7354 (closed Bug: fixed)

Opened 3 years ago

Last modified 3 years ago

Enter key should split a blockquote

Reported by: alfonsoml Owned by: fredck
Priority: Normal Milestone: CKEditor 3.6.2
Component: UI : Enter Key Version:
Keywords: Cc:

Description

Blockquotes are used mainly for replying in mails, in that environment the expected behavior is that when the user press Enter inside the quoted mail, that mail is splitted so he can reply inline to each part.

The proposed patch fixed this issue.

Attachments

7354.patch (1.0 KB) - added by alfonsoml 3 years ago.
Proposed patch
7354.html (4.3 KB) - added by fredck 3 years ago.
Plugin prototype.
7354_2.patch (1.4 KB) - added by fredck 3 years ago.

Change History

Changed 3 years ago by alfonsoml

Proposed patch

comment:1 in reply to: ↑ description Changed 3 years ago by fredck

Replying to alfonsoml:

Blockquotes are used mainly for replying in mails

Sorry, this assumption is wrong. The write way to say this is that, when replaying to e-mails, blockquotes are mainly used to inline the original message.

Blockquote is instead widely used to make reference for text copied from other sources, for poems and long length citations.

Because of this, it's definitely acceptable, and required, to have the ENTER key active inside <blockquote>. If a different behavior is required, it's better to have a plugin that listens to the ENTER key, acting accordingly, or even make it configurable somehow.

So, R-.

comment:2 Changed 3 years ago by alfonsoml

When I'm replying in a forum, or in general quote someone, I usually want an easy way to put my content inline between the quoted text. And when I'm quoting someone I don't edit that text unless it's to add my own text, I don't want my edits appear as part of the original text.

But that's not really important, back to the patch: trying to create it as a plugin seems to complex to me, as other situations it would need to copy lots/all of the code just to perform this little change.

So, what about a config option "enterBreakableElements" that allows to specify a set of tags that would lead to this behavior?

What would be a good name for that option?

comment:3 follow-up: ↓ 4 Changed 3 years ago by protz

So I've given this patch a try, and it does split blockquotes, only if I hold the shift key, though. The text is not immediately inside the blockquote, but rather something like <blockquote><p><pre>the text I'm in</pre></p></blockquote>.

Regarding the use case brought up by fredck, what about restricting this behavior to <blockquote tag="cite">s? The option is fine for me as well. Thanks for addressing this issue!

comment:4 in reply to: ↑ 3 Changed 3 years ago by alfonsoml

Replying to protz:

It would be good to know the source HTML that you are using in order to reproduce exactly the problem. If there are other elements like PRE involved then this can be more complex than the original patch.

If you are pressing the shift key that looks like you have set Enter key to BR and ShiftEnter to P. Is there any special reason about that?
using BR usually leads to more problems than using P

Finally, I think that having a configuration option to state which tags must be splitted is better than hardcoding it to just blockquotes with a custom tag="cite" attribute.

comment:5 Changed 3 years ago by protz

I've put together a small example there.

http://jonathan.protzenko.free.fr/kompose@xulforum.org/content/stub.html

If you position the cursor right after "less often"? in that test case, and hit enter, nothing happens. If you position the cursor in one of the nested blockquotes, and hit enter, nothing happens. Hitting shift-enter sometimes work, but not always. Looks like only shit-enter works, and only in very specific cases.

Please note that I'm running this testcase with no custom ckeditor options, using Firefox 4.

comment:6 Changed 3 years ago by alfonsoml

That HTML includes PRE elements, and the enter key logic in that case it's quite different as it's trying to insert <br> instead of breaking it like any other element, so trying to change that behavior will be far more complex because it can break other current expectations.

Anyway this goes back to previous topics about the enter key and getting out of the current element. It shows that we need some generic way to specify which elements can be broken by the enter key, keeping in mind that it's not the same pressing enter in the middle of an element (to edit something existing) or at the end of it (when we are writting that content).

comment:7 Changed 3 years ago by fredck

Just for reference, this is HTML that can be used to test this issue:

<p>
	Pete Fritchman <petef@mozilla.com> wrote:</petef@mozilla.com></p>
<blockquote type="cite">
	<pre wrap="">
----- Original Message -----
</pre>
	<blockquote type="cite">
		<pre wrap="">
Hey,

This is a cited paragraph at second level.

This is another one.
</pre>
	</blockquote>
	<pre wrap="">
Ok, but this is first level citation</pre>
	<blockquote type="cite">
		<pre wrap="">
Another second level.
</pre>
	</blockquote>
	<pre wrap="">
More at first level.

And even more</pre>
</blockquote>
<p>
	&nbsp;</p>

I'm trying to find out how hard it is to have a plugin for it.

Changed 3 years ago by fredck

Plugin prototype.

comment:8 Changed 3 years ago by fredck

  • Status changed from new to closed
  • Resolution set to wontfix

I'm finding it hard to have a generic configurable solution for this specific case. There are too many variables to be considered and each implementation have its own quirks.

I'm attaching a prototype for a plugin that solves the problem for the Thunderbird Composer needs. I've inlined the plugin definition just to make it simpler to test.

Other implementation on forums or anything else cold be based on this. Or even an attempt to generalize it could be in place (like a plugin that always break cite blockquotes), but in a dedicated plugin anyway.

I'm closing this ticket, as we're not supposed to have this into core at this point. I hope the plugin helps.

comment:9 Changed 3 years ago by fredck

  • Status changed from closed to reopened
  • Resolution wontfix deleted

After some in depth thought and to the benefit of our friends from YAHOO Meme (which don't have a toolbar to remove blockquote), I've come with an in-between solution for this one.

Basically, it works like lists... if you're in an empty paragraph in a blockquote, then ENTER breaks it. This happens at the end of the text as well.

I'll post a patch soon.

Changed 3 years ago by fredck

comment:10 Changed 3 years ago by fredck

  • Status changed from reopened to review
  • Owner changed from alfonsoml to fredck

This patch is supported by dt tests on t/7354:
http://ckeditor.t/dt/plugins/enter/blockquote.html

comment:11 Changed 3 years ago by Saare

  • Status changed from review to review_failed

A similar solution should be found for enterMode BR.

comment:12 follow-up: ↓ 17 Changed 3 years ago by fredck

  • Status changed from review_failed to review

I've been investigating a solution for enterMode BR and it seems it'll be much more complex than the simple enterMode P solution.

I have already coded the tests and a partial patch for it. In any case, I would ask for review of my previous patch again, so we'll have this feature available at least on enterMode P, which is our default and recommended mode.

After closing this ticket, I'll open a new ticket for enterMode BR, attaching the code I have so far, as well as creating the appropriate ticket tests branch.

comment:13 Changed 3 years ago by Saare

  • Status changed from review to review_failed

IE and Opera aren't passing these tests.

Last edited 3 years ago by Saare (previous) (diff)

comment:14 Changed 3 years ago by fredck

  • Status changed from review_failed to review

Sorry, I have all green on IE9, IE9/Compat and Opera 11.50. Can you please recheck it?

comment:15 Changed 3 years ago by Saare

  • Status changed from review to review_passed

I had to clean the cache 3 times to make it pass but it paid off :)

comment:16 Changed 3 years ago by fredck

  • Status changed from review_passed to closed
  • Resolution set to fixed
  • Milestone set to CKEditor 3.6.2

Fixed with [7238].

comment:17 in reply to: ↑ 12 Changed 3 years ago by fredck

Replying to fredck:

After closing this ticket, I'll open a new ticket for enterMode BR, attaching the code I have so far, as well as creating the appropriate ticket tests branch.

I've just opened #8304 for it.

Note: See TracTickets for help on using tickets.
© 2003 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy