Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#7354 closed Bug (fixed)

Enter key should split a blockquote

Reported by: Alfonso Martínez de Lizarrondo Owned by: Frederico Caldeira Knabben
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 (3)

7354.patch (1.0 KB) - added by Alfonso Martínez de Lizarrondo 13 years ago.
Proposed patch
7354.html (4.3 KB) - added by Frederico Caldeira Knabben 13 years ago.
Plugin prototype.
7354_2.patch (1.4 KB) - added by Frederico Caldeira Knabben 13 years ago.

Download all attachments as: .zip

Change History (20)

Changed 13 years ago by Alfonso Martínez de Lizarrondo

Attachment: 7354.patch added

Proposed patch

comment:1 in reply to:  description Changed 13 years ago by Frederico Caldeira Knabben

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 13 years ago by Alfonso Martínez de Lizarrondo

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 Changed 13 years ago by Jonathan Protzenko

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 13 years ago by Alfonso Martínez de Lizarrondo

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 13 years ago by Jonathan Protzenko

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 13 years ago by Alfonso Martínez de Lizarrondo

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 13 years ago by Frederico Caldeira Knabben

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 13 years ago by Frederico Caldeira Knabben

Attachment: 7354.html added

Plugin prototype.

comment:8 Changed 13 years ago by Frederico Caldeira Knabben

Resolution: wontfix
Status: newclosed

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 13 years ago by Frederico Caldeira Knabben

Resolution: wontfix
Status: closedreopened

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 13 years ago by Frederico Caldeira Knabben

Attachment: 7354_2.patch added

comment:10 Changed 13 years ago by Frederico Caldeira Knabben

Owner: changed from Alfonso Martínez de Lizarrondo to Frederico Caldeira Knabben
Status: reopenedreview

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

comment:11 Changed 13 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

A similar solution should be found for enterMode BR.

comment:12 Changed 13 years ago by Frederico Caldeira Knabben

Status: review_failedreview

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 13 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

IE and Opera aren't passing these tests.

Last edited 13 years ago by Sa'ar Zac Elias (previous) (diff)

comment:14 Changed 13 years ago by Frederico Caldeira Knabben

Status: review_failedreview

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

comment:15 Changed 13 years ago by Sa'ar Zac Elias

Status: reviewreview_passed

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

comment:16 Changed 13 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.2
Resolution: fixed
Status: review_passedclosed

Fixed with [7238].

comment:17 in reply to:  12 Changed 13 years ago by Frederico Caldeira Knabben

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 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy