330891 - Composition editor: Return/Enter inserts line break <br>, should insert paragraph break <p>; also implement Shift+Enter for <br>

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, enhancement, P1)

Description

[6e77a 70 6e 6e7a]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: version 1.5 (20051201)

Bug 92686 is still broken in Thunderbird, although it is marked as resolved.

Reproducible: Always

Comment 1

[Magnus Melin [:mkmelin]]

Also see bug 314213 comment 14.

Comment 2

[Mike Cowperthwaite]

xref bug 92686 comment 99.

This is the same in the mail composer for Seamonkey, altho Composer (the HTML file editor) behaves as expected.  Same behavior regardless of the state of the pref: editor.CR_creates_new_p


(In reply to comment #1)
> Also see bug 314213 comment 14.

Unrelated.

Comment 3

[Peter Lairo]

What is the author doesn't want <p> but wants <br>, like when writing an address?

     John Doe
     MyStreet 123
     12345 MyCity

and not

     John Doe

     MyStreet 123

     12345 MyCity

It might be useful to be able to toggle <p> and <br> with a modifier key:

ENTER     = <p>
ALT+ENTER = <br>

Comment 4

[Peter Lairo]

I think the "standard" (MS Word, Google talk) is SHIFT-ENTER = <br>, so:

ENTER       = <p>
SHIFT+ENTER = <br>

Comment 5

[Brian Heinrich]

Why has this behaviour has not yet been implemented in HTML compose mode for e-mail when it's already been implemented in Composer? -- Is it just lack of interest? or is there a potential problem with down-converting from HTML to plain-text?

If/when implemented for HTML e-mail composition, the behaviour should probably, by default, remain turned off; on the basis of this bug, at least, it seems not to be a big enough deal to merit having a UI to toggle the pref . . . but it would certainly be nice to have the behaviour available.

Comment 6

[Magnus Melin [:mkmelin]]

Anyone know why the editor.CR_creates_new_p pref doesn't work in thunderbird?

Comment 7

[Magnus Melin [:mkmelin]]

Daniel, any idea why editor.CR_creates_new_p doesn't seem to work in tb even if one turn it on?

Comment 8

[Magnus Melin [:mkmelin]]

Very much wanted as it's likely to resolve bug 314213 in the process.

Comment 9

[Peter Lairo]

If this bug is implemented, how will the author know how his text will appear at the recipient? 

The author might want vertical space between paragraphs:

bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla 

bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla 

And the recipient might see no space:

bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla 
bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla bla

Text document software (e.g., OpenOffice & MS Word) allows the author to define the spacing between paragraphs (e.g., 1cm or 12pt - both units are sub-optimal, 1em would be better!?) and the (unfortunate) default is to have no spacing. Will Thunderbird allow the author to define the spacing? Will there be a sensible default (i.e., one line height space)?

BTW: And how does/will Thunderbird decide how *much* space to display between paragraphs on *received* e-mails? IMO it should be *one line height*, depending on what font-size the preceding paragraph has.

Comment 11

[Ludovic Hirlimann [:Usul]]

Fabien isn't this a duplicate of the bug you are working on ?

Comment 12

[Ludovic Hirlimann [:Usul]]

xref bug 449243

Comment 13

[(no longer active)]

It definitely depends on it, but since this bug doesn't really list any detailed information, it's hard to tell whether bug 449243 covers all of the cases here or not.

Comment 14

[Fabien Cazenave [:kaze]]

On the current trunk:
 • Shift+Enter always creates a <br>;
 • the editor.CR_creates_new_p pref works fine — at least on Linux.

Ludovic, could you tell whether this pref works on Mac as well?

(In reply to comment #11)
> Fabien isn't this a duplicate of the bug you are working on ?

Probably, yes. But as Ehsan said above, without any sharper test case it’s hard to close this bug.

Comment 15

[Filipe Martins]

This is marked resolved but my Thunderbird 9.0.1 on Windows 7 and on Windows XP couldn't care less about editor.CR_creates_new_p.
At the momment I cant test it on Linux.
I think that at least these bugs should remain open.

Comment 16

[Fabien Cazenave [:kaze]]

Filipe: could you please provide a sharper test case? How can I reproduce the bug you’re experiencing?

Comment 17

[Filipe Martins]

Hello Fabien. Thanks for responding. As a Mozilla fan, its an honor to meet someone of this great project.

I hope you're not talking about unit testing, because I'm really not familiarized with Mozilla's sources.

There's what I have:
  - Thunderbird 9.0.1 on Windows 7 32 bits European Portuguese translation -- same thing happened on XP 32 bits;
  - On Config Editor, I have a boolean key named "editor.CR_creates_new_p" set to true;
  - When writing new messages...

IN «NORMAL» MODE

  - Every return inserts a break, and I get markup like: <body>Paragraph (pressing return now);<br><br></body>;
  - If I press return twice, I get: <body>I'm going to insert two breaks right now<br><br>This is what I got.<br></body>;

IN «PARAGRAPH» MODE

  - Return still inserts breaks <body><p>Paragraph (pressing return now);<br><br></p></body>;
  - Pressing return twice: <body><p>I'm going to insert two breaks right now<br></p><p>This is what I got.<br></p></body>


A few notes:
- I used the Stationary extension to check the source code (https://addons.mozilla.org/pt-PT/thunderbird/addon/stationery/?src=api);
- It seems that there is always an extra, final <br> -- I presume it's put there by Thunderbird itself;
- In Paragraph mode, two returns insert a paragraph with a <br> at the end -- doesn't make sense in my view.

Can I help further?

Comment 18

[Fabien Cazenave [:kaze]]

Hello Felipe, thanks for your time!

The tl;dr version would be:
 • Thunderbird should not ignore the `editor.CR_creates_new_p' pref;
 • it’d be more consistent if Thunderbird started in « paragraph » mode.

I did correct the behavior of [Return] in paragraphs and list items in the core editor, but we’d need to work a bit on the TB front-end side to benefit from it.

(In reply to Filipe Martins from comment #15)
> XP couldn't care less about editor.CR_creates_new_p.

Ugh, right. Confirmed with Thunderbird 8 on Ubuntu Linux.
This pref is used by SeaMonkey Composer and Kompozer but ignored by TB. :-/

(In reply to Filipe Martins from comment #17)
> IN «NORMAL» MODE
>   - Every return inserts a break, and I get markup like: <body>Paragraph
> (pressing return now);<br><br></body>;
>   - If I press return twice, I get: <body>I'm going to insert two breaks
> right now<br><br>This is what I got.<br></body>;

This is not a bug: if the caret is not in a <p> or <li> element, pressing [Return] should create a <br> node, not splitting a <p> or <li>.
However, I agree I’d much prefer Thunderbird to start automatically in « paragraph mode ».

> IN «PARAGRAPH» MODE
>   - Return still inserts breaks <body><p>Paragraph (pressing return
> now);<br><br></p></body>;
>   - Pressing return twice: <body><p>I'm going to insert two breaks right
> now<br></p><p>This is what I got.<br></p></body>

This is the expected behavior: one [Return] creates a <br>, to [Return] create a paragraph.
This is where the `editor.CR_creates_new_p' makes a difference on Composer, but unfortunately this pref is ignored by TB.

> - It seems that there is always an extra, final <br> -- I presume it's put
> there by Thunderbird itself;
> - In Paragraph mode, two returns insert a paragraph with a <br> at the end
> -- doesn't make sense in my view.

Yes, the trailing <br> stuff is a known “feature” of the editor, and we’re aware of that.

> Can I help further?

Thanks again for your time, it seems to me that we’ve closed this bug too quickly. I’ll ping the TB team about it.

Comment 19

[Axel Grude [:realRaven]]

I have taken ages (many hours) to explain to one of my users why Thunderbird does not act like Outlook (read: Word), by inserting style-able child elements into the body tag; This is especially annoying as font choices for the email as a whole turn out to be something very erratic as long as we cannot style direct descendants of body (like body > p, body > div etc.) if any simple [Enter] returns to the "Body Text" style. Default should be "Paragraph" style and this should be created as a default choice on any [Enter]. Only [Shift]+[Enter] should generate a <br>. I wonder how hard it would be to write a patch for this, or whether creating an Addon might be a quicker way to deal with this?

Comment 20

[Axel Grude [:realRaven]]

(In reply to Mike Cowperthwaite from comment #2)
> xref bug 92686 comment 99.
> 
> This is the same in the mail composer for Seamonkey, altho Composer (the
> HTML file editor) behaves as expected.  Same behavior regardless of the
> state of the pref: editor.CR_creates_new_p
> 
I have tried to find the UI for setting this in c-c (it is actually set to the non-default value true in mine), but all I can come up with is this:

http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/pref-composer.xul#33

I cannot find that pane anywhere in the Thunderbird UI, is it missing? Also I can't find references to the preference where it would be used for anything in the processing.

Comment 21

[Axel Grude [:realRaven]]

(In reply to Axel Grude [:realRaven] from comment #20)

> I cannot find that pane anywhere in the Thunderbird UI, is it missing? Also
> I can't find references to the preference where it would be used for
> anything in the processing.

it turns out it is in seamonkey but it is actually not working there either.

Comment 22

[Axel Grude [:realRaven]]

I would like one additional option to extend this behavior to generating new <p> when Enter is hit and the parent is <body> so that an email would start with a <p> as soon as Enter is once hit (force containers, avoid body text). It should also insert a <p> when I hit Enter to split a blockquote.

This behavior would be very similar to what html mail authors are used to in Outlook (behavior which comes from Word). To the majority of these users the Paragraph end mark ¶ (created by hitting Enter) is familiar and signifies the end of a block level element "Paragraph" which maps nicely to <p>. It will improve readability in HTML.

It will also ultimately make it easier to develop  more stable, class based styling which does not affect quoted material. (Once the child selector > is fixed). In this paradigm, Enter always means new paragraph and only Shift-Enter means line break. 

I would have to look at some exceptions for this rules (like creating a line break after an <img>) but generally the rule "if in body, create <p>" and "if in <p>, split into two <p>" would work nicely. As an exception I would also _not_ create the <p> in an empty email until the user hits Enter for the first time (so the edge case of never using the Enter key at all would not generate a <p>) the reason for this is that this won't break any addons that manipulate mail content on notifyComposeBodyReady  (e.g. Stationery). 

Also note that it is not necessary to generate <p> when within another container, but it might still be desired. E.g, should Enter create paragraphs when I am in a <div>?

I believe when in a heading, Enter should close the heading an create a <p> as well - only Shift+Enter should create a <br> in a heading. I will create a more comprehensive table on what should happen based on the current element.

Exceptions
----------
In <pre> Enter should always generate <br> as paragraphs nested within <pre> isn't a good concept.
In <span> (as direct parent) generating or splitting a <p> might not be desired. However if we stick to the rule, should a single backspace undo a the inserted </p><p> split so that the original <span> is restored?

Comment 25

[Thomas D. (:thomas8)]

Suyash might be the hero who can finish this off, and poke the right people to assist...

Comment 26

[Axel Grude [:realRaven]]

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #25)
> Suyash might be the hero who can finish this off, and poke the right people
> to assist...

Is anybody currently working on this? It seems to counter intuitive for so many people who are not used to text only email that we need to hit Enter 2 times in order to create a new paragraph.

Comment 27

[:aceman]

I think this would be something for Jorg.

Comment 28

[Jorg K (CEST = GMT+2)]

Ancient old editor bug (or enhancement request?) from 2006 or even 2001 looking at the predecessor bug 92686. Not sure that I can warm up to it ;-(

Comment 29

[Axel Grude [:realRaven]]

(In reply to Jorg K (GMT+2) from comment #28)
> Ancient old editor bug (or enhancement request?) from 2006 or even 2001
> looking at the predecessor bug 92686. Not sure that I can warm up to it ;-(

Be it ancient or not, it is the ancient people (who are used to word processors) I cannot convince to make proper paragraphs, as they just don't understand why they have to hit [Enter] 2 times.
So I am left with an unreadable mess in HTML, no matter how often I tell them to do this.
Once you get a lot of mails that are a wall of text maybe you can feel my pain :) I wonder, if we introduced <p> as the default in HTML would a lot of people get upset? 
PS: I left my paragraphs the "wrong" way so you get an idea...
Axel

Comment 30

[Wayne Mery (:wsmwk)]

(In reply to Magnus Melin from comment #8)
> Very much wanted as it's likely to resolve bug 314213 in the process.

jorg, unfortunate that you're not feeling cozy with this :)
Do you agree that it's likely to fix bug 314213, which has *many* dupes? 
(plus this one has recent dupes)

(wonder if the shift+enter should be a different bug)

Comment 31

[Magnus Melin [:mkmelin]]

Maybe editor.CR_creates_new_p is working, but we just need to start off in paragraph mode (not body)?

Comment 32

[Jorg K (CEST = GMT+2)]

(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #30)
> jorg, unfortunate that you're not feeling cozy with this :)
Indeed, since it would mean to turn the M-C editor upside down.
There is no support for inserting <p></p> in M-C.
CR_creates_new_p is just wishful thinking, look at a DXR search, there is no support for in in M-C.
I'm not even sure that the M-C folk would let us add support for it.
Actually, this is really an enhancement request.

> > Very much wanted as it's likely to resolve bug 314213 in the process. (comment #8)
> Do you agree that it's likely to fix bug 314213, which has *many* dupes?
It certainly will *not* fix bug 314213. Bug 314213 is about turning DOM to text that is then sent in an e-mail. This is done by the M-C serialisers, I've just added a bit there to fix the serialisation of CJK text (bug 1225864). The M-C serialisers are unowned and apparently in a bad shape, see the white-space copy/paste bug 1214377 (bug 1193153) for which Ehsan accepted a Thunderbird only hack. The real bug 1174452 is still open.

Comment 33

[Magnus Melin [:mkmelin]]

(In reply to Jorg K (GMT+1) from comment #32)
> CR_creates_new_p is just wishful thinking, look at a DXR search, there is no
> support for in in M-C.

There is, it sets editor.returnInParagraphCreatesNewParagraph: http://mxr.mozilla.org/comm-central/ident?i=returnInParagraphCreatesNewParagraph

Comment 34

[Axel Grude [:realRaven]]

(In reply to Magnus Melin from comment #33)
> (In reply to Jorg K (GMT+1) from comment #32)
> > CR_creates_new_p is just wishful thinking, look at a DXR search, there is no
> > support for in in M-C.
> 
> There is, it sets editor.returnInParagraphCreatesNewParagraph:
> http://mxr.mozilla.org/comm-central/
> ident?i=returnInParagraphCreatesNewParagraph

Looks like somebody thought of this before? Happy days :) So the question is if we change this fundmental behavior (start off in <P> mode and keep it sticky with Enter and use a modifier key for BR - I am all for these) should we add a UI to revert to the old behavior? I bet it will feel like "they fixed it!" to most of the HTML users.

Comment 35

[Jorg K (CEST = GMT+2)]

Oops, I didn't see that:
https://dxr.mozilla.org/comm-central/source/editor/ui/composer/content/editor.js#292

Any yes, there is *limited* support in M-C (which doesn't appear to work?):
http://mxr.mozilla.org/comm-central/source/mozilla/editor/composer/nsComposerDocumentCommands.cpp#262
http://mxr.mozilla.org/comm-central/source/mozilla/editor/composer/nsComposerDocumentCommands.cpp#349
http://mxr.mozilla.org/comm-central/source/mozilla/editor/libeditor/nsHTMLEditRules.cpp#6468
I tried setting the preference in TB and it made no difference.

This bug calls for "enter" to result in <p> and "shift enter" to result in <br>, like in the usual Office packages. There is no support for this in M-C.

Making the preference work would not satisfy anyone, since it's not exposed to the user, and even if it were, no one always wants <p> or switch preference all the time.

So a lot of work is required to implement this properly, if at all allowed by M-C, since this would be a TB feature only. No one wants to touch the editor, especially not to support TB features.

I think this bug will not be implemented until we move to a different platform.

Comment 36

[Jorg K (CEST = GMT+2)]

(In reply to Axel Grude [:realRaven] from comment #34)
> modifier key
What's that? Please understand that M-C handles key presses and we'd have to drill open M-C to implement "alternate" behaviour.

Comment 37

[Axel Grude [:realRaven]]

(In reply to Jorg K (GMT+1) from comment #35)
> Oops, I didn't see that:
> https://dxr.mozilla.org/comm-central/source/editor/ui/composer/content/
> editor.js#292
> 
> Any yes, there is *limited* support in M-C (which doesn't appear to work?):
> http://mxr.mozilla.org/comm-central/source/mozilla/editor/composer/
> nsComposerDocumentCommands.cpp#262
> http://mxr.mozilla.org/comm-central/source/mozilla/editor/composer/
> nsComposerDocumentCommands.cpp#349
> http://mxr.mozilla.org/comm-central/source/mozilla/editor/libeditor/
> nsHTMLEditRules.cpp#6468
> I tried setting the preference in TB and it made no difference.
> 
> This bug calls for "enter" to result in <p> and "shift enter" to result in
> <br>, like in the usual Office packages. There is no support for this in M-C.

:(

 
> Making the preference work would not satisfy anyone, since it's not exposed
> to the user, and even if it were, no one always wants <p> or switch
> preference all the time.

I strongly disagree. Not only do office packages work this way, most of the popular HTML editors on Web Sharing platforms that are not directed at a primarily IT-savvy audience such as confluence / sharepoint use <p> as default behavior; <br> is the exception. We have to distinguish between the use cases "html code edit" and "WYSIWYG" HTML content. 
In HTML, it makes complete sense not to have to press Enter twice for a new paragraph.
And a lot of users will expect paragraphs to be properly separated with some space in between them, so:
as a case in point I am replying in the same way my mother does when she uses html Email; I have told her lots of times that she needs to hit Enter twice, but it just doesn't register with her. That the implementation of this is difficult is obviously another page, but don't make roundabout statements like "no one always wants <p>". Shift Enter would be perfectly fine for <br>. 
If you look at your own emails and do an analysis of the mails you have written, tell me how often do you use <br> and how often do you use <br><br>? 
The point I am making here is that in HTML the more often used outcome (paragraph distancing*) should have the easier keyboard shortcut (Enter), and the one that is needed more seldom (single <br>) should have the SHIFT+Enter. This is a purely ergonimic reasoning.
(*) the user probably doesn't care whether this is achieved via <p> or <br><br> but she does about the extras keystrokes.

>> modifier key
>What's that? Please understand that M-C handles key presses and we'd have to drill open M-C to implement "alternate" behaviour.

SHIFT. The problem with totally replacing the editor is that we have not found anybody who would have time to do this; could the Editor Code be forked and moved over to C-C ?

Comment 38

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+1) from comment #35)
> I tried setting the preference in TB and it made no difference.

Setting editor.CR_creates_new_p to true does NOT make a difference for our default format, which is "Body Text". Which is kind of obvious because body text by current UI logic is not same as paragraphs.

Setting editor.CR_creates_new_p to true DOES make a difference when you set your format to "Paragraph".
In paragraph mode, as reported in Comment 17:
- Writing 'foo bar' produces 
> <p>foo bar<br></p>
- pressing Return once after foo inserts <br>:
> <p>foo<br>bar<br></p>
- alternatively pressing Return twice inserts<br></p><p>, thus creating a new paragraph:
> <p>foo<br></p><p>bar<br></p>

Note that trailing <br> is a special animal in current editor, might be needed as a hack to ensure correct behaviour when going to end of lines etc (like blank composition also already has a trailing <br> iirc).

> This bug calls for "enter" to result in <p> and "shift enter" to result in
> <br>, like in the usual Office packages. There is no support for this in M-C.

If we really need M-C to support this because we need/want the feature, we either need to do this ourselves or ask them to do it for us.
But I think keyboard capture could possibly be handled on TB's side; the tricky part is if we can hook that onto present M-C to achieve what we want. We could even try to analyse the situation ourselves and tweak that little bit of HTML directly.

What about this:

When pref is set to true:
- TB starts out composing with 'paragraph mode'==true (instead of 'body text'). Changing existing text already somewhat fails now (preserving <br> at the beginning of following paragraph, where it shows), so we wouldn't be worse I guess.
During composition, if pref==true:
- TB listens for Return key
- for return key event without modifier: send two return key events to the editor
- for return key event with modifier Shift: send one return key event to the editor

So imo this tweak could be possible for TB without touching a single byte of editor.
Expose the pref in UI, make pref==true the new default, done.

> Making the preference work would not satisfy anyone, since it's not exposed
> to the user, and even if it were, no one always wants <p> or switch
> preference all the time.

I agree 100% with all of Axel's comment 37: Getting <p> from single Return keypress is a perfectly reasonable expectation which is ux-consistent with all major word processors (and writing HTML email is essentially basic word processing). And widespread word processors like MS word will teach you very quickly that if you just need a line break (as opposed to paragraph break), you'll have to press Shift+Enter (because the paragraph comes with auto-padding these days which is much harder to get rid of. If we need to expose the pref in the UI for those who got used to the current behaviour, I have no problem with that.

> So a lot of work is required to implement this properly, if at all allowed
> by M-C, since this would be a TB feature only. No one wants to touch the
> editor, especially not to support TB features.

I think you are being too pessimistic on this, and prematurely giving up on exploring possible solutions.

> I think this bug will not be implemented until we move to a different
> platform.

Again, too pessimistic and giving up too early. Pls consider my TB-only tweak above.

Comment 39

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+1) from comment #36)
> (In reply to Axel Grude [:realRaven] from comment #34)
> > modifier key
> What's that? Please understand that M-C handles key presses and we'd have to
> drill open M-C to implement "alternate" behaviour.

I don't like the term 'drill open', imo it sounds prejudiced because it conveyes negative connotations like it's something we shouldn't or couldn't or mustn't do because it's unethical, too complex, or disallowed; whereas if required and agreed-upon, it would just be a normal code change like any other code change. Editor code might be a jungle; some TB code is also a jungle. The fact that this code is organisationally not our territory doesn't mean much as we are one of the main consumers, so we certainly have the right to demand and/or implement changes as long as we don't break other consumers.

In this particular case her, I'd even guess other consumers may want the same.

Comment 40

[Axel Grude [:realRaven]]

Let me modify this somewhat:

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #38)
> (In reply to Jorg K (GMT+1) from comment #35)
> > I tried setting the preference in TB and it made no difference.
> 
> Setting editor.CR_creates_new_p to true does NOT make a difference for our
> default format, which is "Body Text". Which is kind of obvious because body
> text by current UI logic is not same as paragraphs.
> 
> Setting editor.CR_creates_new_p to true DOES make a difference when you set
> your format to "Paragraph".
> In paragraph mode, as reported in Comment 17:
> - Writing 'foo bar' produces 
> > <p>foo bar<br></p>
> - pressing Return once after foo inserts <br>:
> > <p>foo<br>bar<br></p>
> - alternatively pressing Return twice inserts<br></p><p>, thus creating a
> new paragraph:
> > <p>foo<br></p><p>bar<br></p>
> 
> Note that trailing <br> is a special animal in current editor, might be
> needed as a hack to ensure correct behaviour when going to end of lines etc
> (like blank composition also already has a trailing <br> iirc).
> 
> > This bug calls for "enter" to result in <p> and "shift enter" to result in
> > <br>, like in the usual Office packages. There is no support for this in M-C.
> 
> If we really need M-C to support this because we need/want the feature, we
> either need to do this ourselves or ask them to do it for us.
> But I think keyboard capture could possibly be handled on TB's side; the
> tricky part is if we can hook that onto present M-C to achieve what we want.
> We could even try to analyse the situation ourselves and tweak that little
> bit of HTML directly.
> 
> What about this:
> 
> When pref is set to true:
> - TB starts out composing with 'paragraph mode'==true (instead of 'body
> text'). Changing existing text already somewhat fails now (preserving <br>
> at the beginning of following paragraph, where it shows), so we wouldn't be
> worse I guess.
> During composition, if pref==true:
> - TB listens for Return key
> - for return key event without modifier: send two return key events to the
> editor
how about:

if we are in a <p> (at the top level, not a <div> or <span> or <table> within it), insert </p> <p> and set cursor after
if we are not in a <p> insert <p>  </p>  and place cursor in between tags


> - for return key event with modifier Shift: send one return key event to the
> editor
Which should insert a <br> and place cursor after.

Comment 41

[Jorg K (CEST = GMT+2)]

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #38)
> Setting editor.CR_creates_new_p to true DOES make a difference when you set
> ...
> - alternatively pressing Return twice inserts<br></p><p>, thus creating a
> new paragraph:
Amazing. I didn't know that after five years of using TB and almost one year of actively submitting patches.

In paragraph style with editor.CR_creates_new_p set to 'true' I typed:
huhu<enter><enter>huhu<enter><enter>huhu
Result:
<html>
<head></head><body><p>huhu<br></p><p>huhu<br></p><p>huhu<br></p><p><br></p><p>
</p></body>
</html>

Next try:
huhu<shift+enter><shift+enter>huhu<shift+enter><shift+enter>huhu:
Result:
<html>
<head></head><body>huhu<br><br>huhu<br><br>huhu<br><p><br></p><p>
</p></body>
</html>

So basically the editor is already listening to the shift key.

> So imo this tweak could be possible for TB without touching a single byte of
> editor.
I don't know whether it's technically possible to filter keystrokes.

> Pls consider my TB-only tweak above.
I will certainly consider a patch you submit:
- Expose the preference in the user interface.
- If set, start composing with format "paragraph".
That would be a start: If you want <p>, you have to hit <enter> twice.
If you can filter keystrokes and modify them, even better.

Comment 42

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+1) from comment #41)
> (In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment
> #38)
> > Setting editor.CR_creates_new_p to true DOES make a difference when you set
> > ...

I need to correct myself: editor.CR_creates_new_p==true does not change anything at all.
But as soon as you change format to 'paragraph format mode', double return will just effect new <p>s to be created (including trailing <br> which seems useless) or existing <p> to be split up into two.

> > - alternatively pressing Return twice inserts<br></p><p>, thus creating a
> > new paragraph:
> Amazing. I didn't know that after five years of using TB and almost one year
> of actively submitting patches.

Me neither. I just read the previous comments... ;)

> In paragraph style with editor.CR_creates_new_p set to 'true' I typed:
> huhu<enter><enter>huhu<enter><enter>huhu
> Result:
> <html>
> <head></head><body><p>huhu<br></p><p>huhu<br></p><p>huhu<br></p><p><br></
> p><p>
> </p></body>
> </html>

(the above happens with or without the pref set, see above)

> Next try:
> huhu<shift+enter><shift+enter>huhu<shift+enter><shift+enter>huhu:
> Result:
> <html>
> <head></head><body>huhu<br><br>huhu<br><br>huhu<br><p><br></p><p>
> </p></body>
> </html>
> 
> So basically the editor is already listening to the shift key.

Well, we don't know as we are just seeing the net effect. Editor is most likely NOT listening to the shift key, but just listening for plain <return> keys without modifier (to avoid conflicts with shortcuts like ctrl+Enter).

> > So imo this tweak could be possible for TB without touching a single byte of
> > editor.
> I don't know whether it's technically possible to filter keystrokes.

Huh? Of course it's possible to filter keystrokes. We already do that on several elements, e.g. contacts side bar listening for DEL key iirc. Either event listeners like onkeypress or otherwise proper XUL keys, as independent <key> tags or attributes key="..." of some other xul tag. Not sure about performance but as we are already listening for Ctrl+Return in some way, it should be possible.

> > Pls consider my TB-only tweak above.
> I will certainly consider a patch you submit:
> - Expose the preference in the user interface.
> - If set, start composing with format "paragraph".
> That would be a start: If you want <p>, you have to hit <enter> twice.
> If you can filter keystrokes and modify them, even better.

Yes. However, I don't intend to work on this. I think it's fine for both of us to stay out of this; but we should never claim it's too hard or requires extraordinary 'drilling open' just because we haven't tried yet or don't know enough.

Axels comments in this bug are very helpful and he has the right mindset.
Axel, would you give Jorg's idea "patch to submit" of comment 41 a try?

Comment 43

[Jorg K (CEST = GMT+2)]

Attachment: Trying to set the editor flag based on the preference - HAS NO EFFECT

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #42)
> I need to correct myself: editor.CR_creates_new_p==true does not change
> anything at all.
Confirmed. However, there is this code:
https://dxr.mozilla.org/comm-central/source/editor/ui/composer/content/editor.js#292
editor.returnInParagraphCreatesNewParagraph = Services.prefs.getBoolPref(kCRInParagraphsPref);
I don't know in which context this is run.

> Me neither. I just read the previous comments... ;)
That's right, reading helps, I should try it more often ;-)

Look guys, in the patch I'm now setting the editors flag that M-C should obey right at editor initialisation. However, is has no effect.

So I get back to my initial statement:
To implement this properly, one would have to open up the M-C editor.

Comment 44

[Jorg K (CEST = GMT+2)]

I have to correct myself.

Let's read the name of the editor flag properly:
returnInParagraphCreatesNewParagraph, so a return/enter while already in a paragraph creates a new paragraph, not a <br>.

And that works. With my patch and with the preference editor.CR_creates_new_p set to 'true', hitting <enter> while already in a paragraph, that is format "Paragraph", will indeed create a <p> and not a <br>. It's even better. If you hit <shift><return>, you'll get a <br>. How good is that?

So it already works, all you need to do is set the editor flag from the preference.

In format "Body Text" where you're *not* in a paragraph, the option has no effect.

How do you want to proceed:
- Submit this patch only, after removing the dump()
- Change the default value for the preference?
- expose the preference to the UX? Where? How? Wording?
  That we would have to do urgently, since there is a string freeze in a week or so.

Looks like I was 85% wrong and this is an easy win. Well, I observed, that at times the editor is getting confused, so hitting <shift enter> won't advance to the next line. That happens when the <br> that is inserted is the last element in the composition, and if nothing follows, it doesn't cause a visible break, so the cursor doesn't move. But that's an issue that can be fixed in the editor separately.

Comment 45

[Jorg K (CEST = GMT+2)]

Attachment: WIP: Possible solution.

OK, here is a first WIP patch:
1) set editor flag from the preference.
2) set format based on the preference value.
3) set preference default to true.

Number 2) doesn't quite work, since as soon as one starts typing, "Paragraph" jumps back to "Body Text". But that should be fixable.

Comment 46

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+1) from comment #44)
> I have to correct myself.
> 
> Let's read the name of the editor flag properly:
> returnInParagraphCreatesNewParagraph, so a return/enter while already in a
> paragraph creates a new paragraph, not a <br>.
> 
> And that works. With my patch and with the preference
> editor.CR_creates_new_p set to 'true', hitting <enter> while already in a
> paragraph, that is format "Paragraph", will indeed create a <p> and not a
> <br>.

So setting the editor flag does really change the behaviour?

Because just flipping the pref does absolutely nothing:
I'm using 45.0a1 (2015-11-28) on Win10.
Changing only the pref value does not change anything of the following behaviour when in 'paragraph mode' (not body text):
You need to hit <enter> TWICE within existing <p> to create another one, and it will always be <p>bla<br></p> (including trailing <br>.

> It's even better. If you hit <shift><return>, you'll get a <br>. How
> good is that?
> 
> So it already works, all you need to do is set the editor flag from the
> preference.
> 
> In format "Body Text" where you're *not* in a paragraph, the option has no
> effect.
> 
> How do you want to proceed:
> - Submit this patch only, after removing the dump()

That's the minimal solution we can fall back to without strings.

> - Change the default value for the preference?

I'm in favor of that if it works as you described.

> - expose the preference to the UX? Where? How? Wording?

I'm not 100% sure, but I guess that's needed because we've trained users for ages that you need two <returns> for making a paragraph, so some might be annoyed if it changes, even though the new behaviour is much superior and more consistent with any word processors and many HTML document editors.

Tools > Options > Composition > General > HTML (wireframe) > (below colors)
[x] Return key creates a new paragraph

Just first thoughts, no deep considerations as yet.

>   That we would have to do urgently, since there is a string freeze in a
> week or so.
> 
> Looks like I was 85% wrong and this is an easy win.

See, told ya ;)

Comment 47

[Thomas D. (:thomas8)]

When fixing this bug, at least when making the new and better behaviour the default, we must pay a little attention to how our plaintext conversion routines handle this, otherwise the plaintext of HTML messages with just <p>s and no <br>s might look odd. So plaintext conversion probably needs to add two line breaks where it encounters <p>s.

Comment 48

[Jorg K (CEST = GMT+2)]

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #46)

> So setting the editor flag does really change the behaviour?
Yes, aren't you the master of playing with the onmi.ja? Try it, the change is in a JS file.

> Because just flipping the pref does absolutely nothing:
> You need to hit <enter> TWICE within existing <p> to create another one, and
> it will always be <p>bla<br></p> (including trailing <br>.
Try the patch!

> Just first thoughts, no deep considerations as yet.
Awaiting the UX man's deep consideration ;-)

> See, told ya ;)
Happy to stand corrected.

Comment 49

[Jorg K (CEST = GMT+2)]

As for plain text conversion:

This:
<html>
<head></head><body bgcolor="#FFFFFF" text="#000000"><p>huhu</p><p>huhu</p><p>huhu<br></p></body>
</html>

gets turned into this:
huhu

huhu

huhu

All works!

Comment 50

[Magnus Melin [:mkmelin]]

Comment on attachment 8696036 [details] [diff] [review]
WIP: Possible solution.

Review of attachment 8696036 [details] [diff] [review]:
-----------------------------------------------------------------

Yes! Comment 31 :)

Maybe put editor.CR_creates_new_p true in all-thunderbird.js instead. Though OTOH, I don't see a reason why it shouldn’t be the default there too.

I wouldn't think this is something that requires a UI pref.

Comment 51

[Joe Sabash [:JoeS1]]

Amazing :) :) after years of pasting </p> <p> into raw html, I find an old dog CAN learn new tricks. Never thought of pressing enter twice. This is on the right track Kudos to all

Comment 52

[Jorg K (CEST = GMT+2)]

Thanks for the quick feedback.

(In reply to Magnus Melin from comment #50)
> Yes! Comment 31 :)
You've been doing this longer than I have, so I'm happy to admit that you were right and I was wrong. However, I had the initiative to explore it further. That's teamwork ;-)

> Maybe put editor.CR_creates_new_p true in all-thunderbird.js instead. Though
> OTOH, I don't see a reason why it shouldn’t be the default there too.
Well, Alex tells us that <enter> giving <p></p> and <shift enter> giving <br> is the expected behaviour, so after all these years we can now do what's expected.

> I wouldn't think this is something that requires a UI pref.
I let Thomas opine on that. Sure, it's less work without a change to the UI.

On another note. I looked a bit at the "cmd_paragraphState" issue. Setting the state to "p" doesn't stick since to command state is set by the M-C editor. When you first enter text, you get this:
<html>
<head></head><body bgcolor="#FFFFFF" text="#000000"><font face="Aharoni">Magnus</font></body>
</html>
and since there is no <p> anywhere to be seen, the editor feeds back, that we are *not* in paragraph mode.

It's different when you switch to paragraph mode from the menu, then you get this:
<html>
<head></head><body bgcolor="#FFFFFF" text="#000000"><p><br></p></body>
</html>

If we really want to default to paragraph format to start with, I have to find a way to make this stick. If push comes to shove (as they say in Australia), we'd have to insert <p></br></p> into the editor. Then the setting will stick (I hope).

Perhaps our editor expert Neil can give me a hint.

Neil: Please look at the patch and tell me how I can make the paragraph format stick.

Comment 53

[Thomas D. (:thomas8)]

(In reply to Jorg K (GMT+1) from comment #52)
> Thanks for the quick feedback.
> 
> (In reply to Magnus Melin from comment #50)
> > Yes! Comment 31 :)
> You've been doing this longer than I have, so I'm happy to admit that you
> were right and I was wrong. However, I had the initiative to explore it
> further. That's teamwork ;-)

Great teamwork indeed Jorg, thanks!

> > Maybe put editor.CR_creates_new_p true in all-thunderbird.js instead. Though
> > OTOH, I don't see a reason why it shouldn’t be the default there too.
> Well, Alex tells us that <enter> giving <p></p> and <shift enter> giving
> <br> is the expected behaviour, so after all these years we can now do
> what's expected.
> 
> > I wouldn't think this is something that requires a UI pref.
> I let Thomas opine on that. Sure, it's less work without a change to the UI.

It should really have been the expected behaviour to begin with, so maybe we can get away without UI.
Per-message change is simple, just change back to 'Body Text' mode.
Only permanent/default change would require setting the hidden pref (also possible if you really need it).

However, please note that there might be users out there whose word processors default to paragraphs without vertical padding, so they also use paragraph marks where there should really be line breaks.
So to make paragraphs, they'll use two paragraph marks to create the vertical spacing.
Until Word 2003 (at least), paragraphs without vertical padding was still default behaviour (please correct me if I'm wrong).
And my Word 2003 still runs amazingly well (although newer versions do come with their own advantages).

> On another note. I looked a bit at the "cmd_paragraphState" issue. Setting
> the state to "p" doesn't stick since to command state is set by the M-C
> editor. When you first enter text, you get this:
> <html>
> <head></head><body bgcolor="#FFFFFF" text="#000000"><font
> face="Aharoni">Magnus</font></body>
> </html>
> and since there is no <p> anywhere to be seen, the editor feeds back, that
> we are *not* in paragraph mode.
> 
> It's different when you switch to paragraph mode from the menu, then you get
> this:
> <html>
> <head></head><body bgcolor="#FFFFFF" text="#000000"><p><br></p></body>
> </html>
> 
> If we really want to default to paragraph format to start with, I have to
> find a way to make this stick. If push comes to shove (as they say in
> Australia), we'd have to insert <p></br></p> into the editor. Then the
> setting will stick (I hope).
> 
> Perhaps our editor expert Neil can give me a hint.
> 
> Neil: Please look at the patch and tell me how I can make the paragraph
> format stick.

Comment 54

[Peter Lairo]

(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #53)

> However, please note that there might be users out there whose word
> processors default to paragraphs without vertical padding, so they also use
> paragraph marks where there should really be line breaks.
> So to make paragraphs, they'll use two paragraph marks to create the
> vertical spacing.
> Until Word 2003 (at least), paragraphs without vertical padding was still
> default behaviour (please correct me if I'm wrong).
> And my Word 2003 still runs amazingly well (although newer versions do come
> with their own advantages).

I think this might be the case for almost all users, since IIRC the current versions of Word (and for sure LibreOffice and OpenOffice) have NO vertical padding for paragraphs by default.

Things to consider:

- How does the vertical spacing of <p>bla</p><p>bla</p> appear while COMPOSING an e-mail?

- How does the vertical spacing of <p>bla</p><p>bla</p> appear when READING an incoming e-mail?

Should Thunderbird offer a (UI or hidden) pref to define the default vertical spacing (e.g. 1em)?

- How does the vertical spacing of <p>bla</p><p>bla</p> appear on OTHER mail clients?

Should Thunderbird *include CSS* in the sent e-mail that defines the vertical spacing, since most other mail clients will likely not have any vertical paragraph spacing, and thus making "our" paragraphs look bunched up against each other? (see my comment #9)

- How should we handle cases where others send us e-mails with TWO paragraphs (because their client has no vertical <p>-spacing but they - sensibly - wanted some). We shouldn't show DOUBLE spacing! Should we silently ignore the superfluous <p>?

<p>First paragraph</p>
<p></p>                     <-- extra <p>
<p>Second paragraph</p>

Bad behavior:
+-----------------------+
| First paragraph       |
|                       |
|                       |   <-- too much vert space
| Second paragraph
+-----------------------+

Good behavior:
+-----------------------+
| First paragraph       |
|                       |
| Second paragraph      |
+-----------------------+

Comment 55

[Jorg K (CEST = GMT+2)]

Attachment: WIP: Possible solution (take 2)

This works now.
If editor.CR_creates_new_p is set to 'true', two things happen:
- we set the editor.returnInParagraphCreatesNewParagraph flag.
  That will then create <p> when hitting <enter>, <br> with shift enter.
- we switch the format to "Paragraph". To make the setting stick we
  insert <p><br></p>. Sadly this "dirties" the document, so when you close it,
  you need to confirm. I'll see whether I can "un-dirty" it.

Comment 56

[Jorg K (CEST = GMT+2)]

Attachment: Possible solution (v3).

OK, "un-dirtying" the document now. So this now works:
If editor.CR_creates_new_p is set to 'true', two things happen:
- we set the editor.returnInParagraphCreatesNewParagraph flag.
  That will then create <p> when hitting <enter>, <br> with shift enter.
- we switch the format to "Paragraph".

Comment 57

[Axel Grude [:realRaven]]

(In reply to Jorg K (GMT+1) from comment #56)
> Created attachment 8696219 [details] [diff] [review]
> Possible solution (v3).
> 
> OK, "un-dirtying" the document now. So this now works:
> If editor.CR_creates_new_p is set to 'true', two things happen:
> - we set the editor.returnInParagraphCreatesNewParagraph flag.
>   That will then create <p> when hitting <enter>, <br> with shift enter.
> - we switch the format to "Paragraph".

+1 (not compiled, but if it works like that...)
You are a genius and I will love you forever if you can make this land in Tb 43.0 :)

Comment 58

[Jorg K (CEST = GMT+2)]

This won't land on TB 43, since TB 45 is next. I'm aiming for that, but reviews are sometimes slooooooow.

Now I've been thinking that we should *not* change existing behaviour. I'm proposing to leave the preference set to false, but expose it to the UI as per Thomas' suggestion:

Tools > Options, Composition, General: HTML, (below colors put)
[x] Use paragraph format by default, return key creates a new paragraph

Adventurous users then set this option once in their lives. All others continue they way they have been before. I'd hate to think that users would now create all these <p> with unforeseen complications.

Do we all agree?

Comment 59

[Axel Grude [:realRaven]]

(In reply to Jorg K (GMT+1) from comment #58)
> This won't land on TB 43, since TB 45 is next. I'm aiming for that, but
> reviews are sometimes slooooooow.
> 
> Now I've been thinking that we should *not* change existing behaviour. I'm
> proposing to leave the preference set to false, but expose it to the UI as
> per Thomas' suggestion:
> 
> Tools > Options, Composition, General: HTML, (below colors put)
> [x] Use paragraph format by default, return key creates a new paragraph
> 
> Adventurous users then set this option once in their lives. All others
> continue they way they have been before. I'd hate to think that users would
> now create all these <p> with unforeseen complications.

Personally I would prefer (in HTML mode only, right?) for us to be "bold" (Firefox style) and default to the new behavior (*based on my assumption that the vast majority of users has millions of <BR><BR> in their markup in order to "simulate" paragraphs and the the need for a single <br> is an absolute exception) use this as s signal that there will be more productivity changes in Composer in the future. But if the majority votes for the old behavior, even just having a checkbox in options for v 45.0 would be a huge step forward.

(*) I admit this is an assumption but I would seriously likel to challenge any HTML user to show me emails where the single <br> is in the majority. Obviously we must not insert <p> if we are in a <pre> or <div> or <tt> or <code> section as a direct parent, so logic may be a little more complicated. Is this the way this would work? Or does it complicate the implementation of this?


Consider the following examples, and my suggestion for "ideal" behavior:

Example 0:
<p>  bla bla {cursor}  </p>

Enter should result in 

<p>  bla bla  </p>
<p>  {cursor}  </p>



Example 1:
<p>  bla bla <font class=x> 123 {cursor} 456  </font>  </p>

Enter should result in 

<p>  bla bla <font class=x> 123 </font>  </p>
<p><font class=x> {cursor} 456 </font>  </p>

Example 2:
<p>  bla bla <div class=x> {cursor}   </div>  </p>

Enter should result in:
<p>  bla bla <div class=x> <br> {cursor}   </div>  </p>

The reason for example 2 would if you create a block container within a paragraph you would not expect that to be "broken up" into 2 block container and duplication of paragraphs. However:

Example 3:
<div>  <p>  123 {cursor} 456  </p>  </div>

Then Enter should:
<div>  <p> 123 </p>  
       <p> {cursor} 456 </p>  
</div>

So the immediate parent (and font processing would have to be skipped in evaluation) decides whether this becomes a </p><p> or a <br>

Last example; a span within a paragraph gets broken by the paragraph break:

Example 4:
<p> <span class=x> 123 {cursor} 456 </span>  </p>

Enter:
<p> <span class=x> 123 </span>  </p>
<p> <span class=x> {cursor} 456 </span>  </p>

Comment 60

[Jorg K (CEST = GMT+2)]

Attachment: Proposed solution (v4).

Sorry about the SPAM, I only need one review, so whoever comes first ...

This patch does the following:
- Exposes preference editor.CR_creates_new_p in the user interface,
  screenshot will be attached. Default unchanged set to 'false'.
- If set, two things happen:
  + editor.returnInParagraphCreatesNewParagraph flag is set.
    That will then create <p> when hitting <enter>,
    <br> with <shift enter>.
  + we switch the format to "Paragraph".

This is all I can offer. I repeat: We expose the existing M-C functionality. Nothing more.

So please no discussions on what would be nice or desirable, please no discussions on vertical spacing or CSS. This is an C-C patch only, no change to existing M-C functionality.

Comment 61

[Jorg K (CEST = GMT+2)]

Attachment: Screenshot of the new option.

Sorry Richard, I stuck the ui-r? onto the screenshot.
Anyway, you find a description of what is planned in the previous comment.

Comment 62

[Richard Marti (:Paenglab)]

Comment on attachment 8696263 [details]
Screenshot of the new option.

When the reviewer wants this pref in UI then ui-r+

The text is a bit longish but I can't propose a better one. I hope it will be not too long in other locales which would move the buttons on the right out of the view.

Comment 63

[Jorg K (CEST = GMT+2)]

Thomas suggested "[x] Return key creates a new paragraph".
At least is should be: Enter key creates a new paragraph.

Comment 64

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+1) from comment #63)
> Thomas suggested "[x] Return key creates a new paragraph".
> At least is should be: Enter key creates a new paragraph.

This would be better.

Comment 65

[Jorg K (CEST = GMT+2)]

Attachment: Video of the proposed new functionality.

Comment 66

[Magnus Melin [:mkmelin]]

Comment on attachment 8696259 [details] [diff] [review]
Proposed solution (v4).

Review of attachment 8696259 [details] [diff] [review]:
-----------------------------------------------------------------

I'd say we want this on by default.

A few minor issues
 - for "on replies start my reply after the quote" mode, "after the quote" is body
 - for forward (inline) you get an extra paragraph (I think?) - and there's 3 annoying newlines  on top to begin with... + some after the headers (but this might be for another bug though)

Not sure if these are easily fixable.

::: mail/components/compose/content/MsgComposeCommands.js
@@ +4866,5 @@
> +  if (useP) {
> +    // Select paragraph format by default.
> +    SetMsgBodyFrameFocus();
> +    let pElement  = editor.createElementWithDefaults("p");
> +    let brElement = editor.createElementWithDefaults("br");

I'd refrain from double space to align. Such cases really get out of line when later someone for some reason adds a third line...

Comment 67

[Jorg K (CEST = GMT+2)]

Comment on attachment 8696259 [details] [diff] [review]
Proposed solution (v4).

Thanks for looking at this so quickly.

(In reply to Magnus Melin from comment #66)
> I'd say we want this on by default.
Are you really really really sure?
You want to change the behaviour of the system for everyone to something new and unexpected?
I really really really would *not* do that. Whoever cares will find the option. As I said, there are minor issues with the M-C editor. An inserted <br> does not show when nothing follows, so the cursor remains at the end of the line where you typed the <shift enter> and moves to the next line when you hit the next key. So I really don't want "normal" users to suffer from any side-effects.

> A few minor issues
>  - for "on replies start my reply after the quote" mode, "after the quote"
>    is body
>  - for forward (inline) you get an extra paragraph (I think?) - and there's
>    3 annoying newlines  on top to begin with... + some after the headers (but
>    this might be for another bug though)
> Not sure if these are easily fixable.
I've got to look at this. I'm blindly inserting the <p><br></p> to keep the paragraph format current. Obviously that's good for a new e-mail but not for a reply/forward.

> I'd refrain from double space to align. Such cases really get out of line
> when later someone for some reason adds a third line...
Sure.

Updated patch in the next 24 hours.

Comment 68

[Jorg K (CEST = GMT+2)]

OK, I tried reply above and below the quote. In both cases I end up with "Body Text" and the format selector is broken. I can't choose anything else. Do you see that also?

On a inline forward (without the patch), I get three blank lines. With the patch, I get one <p> and then the tree blank lines in "Body Text".

Obviously reply and forward processing is different to creating a new message.

Maybe it's wishful thinking to force the format to Paragraph to start with since there are too may complications for the three modes: New, Reply above/below, forward.

Well, the New already works, for inline forward I'd have to look where the three empty lines are inserted and just not insert them any more, and Reply needs further investigation.

Comment 69

[Jorg K (CEST = GMT+2)]

Turns out that it was a real bad idea to insert the <p> in MsgComposeCommands.js. The processing needs to go into nsMsgCompose::BuildBodyMessageAndSignature() or nsMsgCompose::ConvertAndLoadComposeWindow(). There are complication with signatures, etc. :-(

Comment 70

[Magnus Melin [:mkmelin]]

(In reply to Jorg K (GMT+1) from comment #67)
> Are you really really really sure?

I think we should have it on by default at least for now, and depending on feedback during alpha+beta we could always change the default.

> You want to change the behaviour of the system for everyone to something new
> and unexpected?

I think it's just new, but better :)

> I really really really would *not* do that. Whoever cares will find the
> option. 

The vast majority just use defaults I'd think. I see lines crammed together in replies quite often when they aren't expected to. It's not obvious to anyone this is the cause of that.

> As I said, there are minor issues with the M-C editor. An inserted
> <br> does not show when nothing follows, so the cursor remains at the end of
> the line where you typed the <shift enter> and moves to the next line when
> you hit the next key. So

FWIW, I don't seem to repro this (on linux).

Comment 71

[Jorg K (CEST = GMT+2)]

This stuff is fiendishly difficult. I've been experimenting in nsMsgCompose::ConvertAndLoadComposeWindow() all morning.

There are these test cases:
1) New message.
2) New message with signature.

3) Reply above quote.
4) Reply above quote with signature above.
5) Reply above quote with signature below.
6) Reply below quote.
7) Reply below quote with signature below.

8) Forward. 
9) Forward with signature above.
10) Forward with signature below.

So far, I have the first five working. I'll attach a WIP soon.

(In reply to Magnus Melin from comment #70)
> FWIW, I don't seem to repro this (on linux).
Not with the patch I attached, but I can see this playing around.

Comment 72

[Jorg K (CEST = GMT+2)]

Attachment: WIP (v5): test cases 1) to 7) work, forward does't not work.

Comment 73

[Jorg K (CEST = GMT+2)]

In fact, the forward processing works in principle, only that some other part of the system inserts another blank line at the front after we're already done, so the paragraph I insert is in the second line. Also, forward inserts four empty lines after the inlined forwarded text, even if not appending the signature. This is another bug but it needs to be fixed here since otherwise the paragraph is not selected properly.

Comment 74

[Jorg K (CEST = GMT+2)]

Found the problem: nsHTMLEditor::BeginningOfDocument() positions inside the "forward div":

<html>
<head></head><body bgcolor="#FFFFFF" text="#000000"><br> <=== nasty <br>
<div class="moz-forward-container">
<p><br></p> <=== I insert.
<pre class="moz-signature" cols="72">**Signature**</pre>
<br><br>-------- Forwarded Message --------

Even the signature gets placed into that div, that's already wrong.

Comment 75

[Axel Grude [:realRaven]]

(In reply to Jorg K (GMT+1) from comment #71)
> This stuff is fiendishly difficult. I've been experimenting in
> nsMsgCompose::ConvertAndLoadComposeWindow() all morning.

I know. I have written the Addon SmartTemplate4 which pretty much tears aparts the whole email and puts it back together again in order to apply nicer "intelligent" headers and there is a lot of hidden complexity here.

However: shouldn't whatever "Enter" does only depend on 2 things?

the parent element container chain (and quote might be part of this chain)
the immediate parent element (which might be a <div> <p> <tt> <h*> <td> <th> or <span>)
There should be certain parent elements which result in the old behavior <br> and a small few where it should be <p></p>  or </p><p>.

For instance if I write in a  <h1> abcdef {cursor} </h1> and I am at the end I would expect it to jump to the next line and great a new paragraph.

<h1>abcdef </h1>
<p> {cursor} </p>

For reply within quote: Should a simple Enter within a quoted block always simply break the quote in 2 and insert a paragraph?

<blockquote>  abcde .. more tags, bla bla  {cursor}  ...more stuff here </blockquote>

Enter:

<blockquote> abcde .. more tags, bla bla   (close all tags) </blockquote>
<p>{cursor}</p>
<blockquote> (reopen closed tags from previous quote block) ...more stuff here  </blockquote>

What is the current behavior in this case? I would think it does the same but inserts just a <br>


> 
> There are these test cases:
> 1) New message.
> 2) New message with signature.
> 
> 3) Reply above quote.
> 4) Reply above quote with signature above.
> 5) Reply above quote with signature below.
> 6) Reply below quote.
> 7) Reply below quote with signature below.
> 
> 8) Forward. 
> 9) Forward with signature above.
> 10) Forward with signature below.
> 
> So far, I have the first five working. I'll attach a WIP soon.
> 
> (In reply to Magnus Melin from comment #70)
> > FWIW, I don't seem to repro this (on linux).
> Not with the patch I attached, but I can see this playing around.

Comment 76

[Jorg K (CEST = GMT+2)]

Attachment: WIP (v6)

OK, the compose logic in nsMsgCompose::ConvertAndLoadComposeWindow() is an absolutely deadly mess. I was messing around with all day until I realised that for forwarded inline messages, more processing is done in cloudAttachmentLinkManager.js, gCloudAttachmentLinkManager.NotifyComposeBodyReady. What a gloriously wrong name to process new, replied and forwarded messages.

Turns out that in there the line break was added before my carefully crafted paragraph. I took that out, but since the whole message is wrapped into a <div class="moz-forward-container"> also the new composition text would end up in that div.

So for forwarding, it's not a good idea to process the paragraph adding in code, but to do it in the JS code. At the same time it would most likely be better to move all the processing, also for new and replied messages into the JS code and remove it from the C++ code which lives in mailnews and therefore affects S/M.

Gentlemen what do you think. Setting the format to "Paragraph" automatically is fiendishly difficult, since for it to stick I actually must insert a paragraph. Should it be done in C++ or JS?

I don't see a chance to land the bug this week before the branch date.

So I suggest to land the string change:
+<!ENTITY enterKeyCreatesNewParagraph.label    "When using paragraph format, the enter key creates a new paragraph">
+<!ENTITY enterKeyCreatesNewParagraph.accesskey "p">
and do the rest during the Aurora cycle.

We could also restrict the setting of the paragraph style to new messages, since there we don't have the mess and variations of replies/quotes below/above, inline forwards and signatures below/above.

Opinions?

Alex: PLEASE, I don't have time for detail discussions, so I will your have to ignore your comment. This bug is about enabling the existing M-C functionality in TB, nothing more. It's hard enough to do this as I found out.

Comment 77

[Axel Grude [:realRaven]]

(In reply to Jorg K (GMT+1) from comment #76)
> Created attachment 8696532 [details] [diff] [review]
> WIP (v6)
> 
> OK, the compose logic in nsMsgCompose::ConvertAndLoadComposeWindow() is an
> absolutely deadly mess. I was messing around with all day until I realised
> that for forwarded inline messages, more processing is done in
> cloudAttachmentLinkManager.js,
> gCloudAttachmentLinkManager.NotifyComposeBodyReady. What a gloriously wrong
> name to process new, replied and forwarded messages.

That's what we are using in our Addon SmartTemplate4, and the Stationery Addons uses that as well.

> Gentlemen what do you think. Setting the format to "Paragraph" automatically
> is fiendishly difficult, since for it to stick I actually must insert a
> paragraph. Should it be done in C++ or JS?

I am for JS (as it easier to extend with Addons code)

> 
> I don't see a chance to land the bug this week before the branch date.
> 
> So I suggest to land the string change:
> +<!ENTITY enterKeyCreatesNewParagraph.label    "When using paragraph format,
> the enter key creates a new paragraph">
> +<!ENTITY enterKeyCreatesNewParagraph.accesskey "p">
> and do the rest during the Aurora cycle.
+1

Comment 78

[Jorg K (CEST = GMT+2)]

Thanks for the feedback.

I'll go ahead and scrap the changes to the C++ code (a day's work of experimenting down the drain). Then I'll prepare a minimal patch what will work for "new" and perhaps some other cases to land before the branch date and fix the rest in the coming weeks.

Comment 79

[Jorg K (CEST = GMT+2)]

Attachment: WIP (v7)

OK, I scrapped the C++ changes, now it's all done in JS in cloudAttachmentLinkManager.js, what a jolly good name!

Forward and new should work. They are easy since the collapsed selection (cursor) is at the very top and I can just add my <p> there. In case of forward this now ends up before the <div class="moz-forward-container"> which is good. Note that there are two ugly breaks in the <div class="moz-forward-container">. Maybe I should hunt them down and kill at least one of them.

Reply processing is more tricky since we can reply below and there are signatures. However, we get the selection delivered at the right spot, so with luck it can be done easily.

To be continued.

Comment 80

[Jorg K (CEST = GMT+2)]

Attachment: WIP (v7b)

Moved the ugly state setting out of MsgComposeCommands.js and to where it belongs in cloudAttachmentLinkManager.js so the processing is kept all in the one place.

Reply still missing.

Comment 81

[Jorg K (CEST = GMT+2)]

Attachment: Screenshot of the new option (reworded).

Richard, sorry, I changed my mind about the wording.

I think it's very important to communicate that this function will *only* work in paragraph mode. I hope you agree.

Comment 82

[Richard Marti (:Paenglab)]

Before, the paragraph mode could be set as default and now when the user selects paragraph mode this pref is respected, right?

Comment 83

[Jorg K (CEST = GMT+2)]

No. I still do my very best to set the the format to paragraph for new, forwarded and replied e-mail when the preference is set (almost working, reply still to implement, you could try it).

However, the user can click into "Body Text" and then hitting "enter" won't have the effect of inserting <p>.

The full option would read:
"(1) Use paragraph format by default, (2) the enter key creates a new paragraph, (3) when using paragraph format".

Since we don't want to write too much, so I prefer to not say (1) but to say (3) since it's not obvious - at least not to me - that it only works in paragraph format. So my suggestion therefore is:

"When using paragraph format, the enter key creates a new paragraph".

The alert user will see that paragraph is indeed set.

Comment 84

[Jorg K (CEST = GMT+2)]

Attachment: Full solution (v8).

All working: New, reply and forward. All done in JS.

Comment 85

[Richard Marti (:Paenglab)]

Comment on attachment 8696616 [details]
Screenshot of the new option (reworded).

After some playing this looks like the right way.

ui-r+ when you insert between the color hbox and the new pref a separator class="thin".

Comment 86

[Jorg K (CEST = GMT+2)]

Will do in the next round, thanks!

Comment 87

[Magnus Melin [:mkmelin]]

Comment on attachment 8696658 [details] [diff] [review]
Full solution (v8).

Review of attachment 8696658 [details] [diff] [review]:
-----------------------------------------------------------------

Great! Seems to work AFAIKT. r=mkmelin

::: editor/ui/composer.js
@@ +94,5 @@
>  pref("editor.dont_lock_spell_files", true);
>  #endif
>  #endif
>  
> +pref("editor.CR_creates_new_p",      true);

no extra spaces needed (though some were there originally too)

::: mail/components/compose/content/cloudAttachmentLinkManager.js
@@ +76,5 @@
> +    if (gMsgCompose.composeHTML && useParagraph) {
> +      let editor = GetCurrentEditor();
> +      editor.enableUndo(false);
> +
> +      let pElement  = editor.createElementWithDefaults("p");

double space (here and the other place)

Comment 88

[Jorg K (CEST = GMT+2)]

Attachment: Full solution (v8b).

Carrying over Magnus' r+.
Fixed nits (white-space problems).

Comment 89

[Jorg K (CEST = GMT+2)]

Thanks for the quick review!

Comment 90

[Jorg K (CEST = GMT+2)]

Sorry, no checkin yet, I forgot Richard's wish. Coming up.

Comment 91

[Jorg K (CEST = GMT+2)]

Attachment: Full solution (v8b) - accidentally attached again.

Carrying forward Magnus' r+.
Carrying forward Richard's ui-r+.
Sorry, I forgot to add the separator. Now I did.

Comment 92

[Jorg K (CEST = GMT+2)]

Attachment: Full solution (v8c).

Grrr. Forgot the "hg qref".

Comment 93

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/322cdc163b997fc08b3f61e87149466eefee22ff
Bug 330891 - Initialise editor with editor.CR_creates_new_p preference. r=mkmelin

Comment 94

[aleth [:aleth]]

Looks like this may have busted some mozmill tests. Please investigate.

Comment 95

[aleth [:aleth]]

In fact, there is no "maybe" about it:
 EXCEPTION: The attachment URL containment node should be preceded by a linebreak: 'p' != 'br'.

Comment 96

[Jorg K (CEST = GMT+2)]

Good Lord, that breaks a bunch of tests:

TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_inserts_linebreak_on_empty_compose
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_inserts_linebreak_on_empty_compose_with_signature
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_written_message
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_empty_reply_above
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_nonempty_reply_above
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_empty_reply_below
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_nonempty_reply_below
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_empty_forward
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_adding_filelinks_to_forward
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/cloudfile/test-cloudfile-attachment-urls.js | test-cloudfile-attachment-urls.js::test_insertion_restores_caret_point
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-forward-headers.js | test-forward-headers.js::test_forward_inline
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-forward-headers.js | test-forward-headers.js::test_forward_as_attachments
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-charset-upgrade.js | test-charset-upgrade.js::test_encoding_upgrade_html_compose
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-charset-upgrade.js | test-charset-upgrade.js::test_encoding_upgrade_plaintext_compose
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-signature-updating.js | test-signature-updating.js::testHTMLComposeWindowSwitchSignatures
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-signature-updating.js | test-signature-updating.js::testHTMLComposeWindowSwitchSignaturesWithSuppressedSeparator

I'll fix the tests, don't worry ;-) All I'll do is set the preference back to the previous default in each test, OK?
Give me a few hours.

Comment 97

[aleth [:aleth]]

Please don't just set the pref back to the previous value without checking carefully that you haven't in fact broken what those tests are testing when the pref is set!

Comment 98

[Jorg K (CEST = GMT+2)]

Summary:
test-cloudfile-attachment-urls.js - 10 failures
test-forward-headers.js - 2 failures
test-charset-upgrade.js - 2 failures
test-signature-updating.js - 2 failures

All I've changed is inserting a <p><br></p> instead of a <br> or inserting a <p><br></p> instead of nothing.

The tests don't lend themselves to testing x or y, sorry. So for now, I will set the preference so that the tests work again.

Comment 99

[Jorg K (CEST = GMT+2)]

Attachment: Fixing the tests.

Try run here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=48f88bc2c668
This should fix all tests, or perhaps one will still fail since I removed code that added a <br> after an inline forward to reduce unnecessary white-space:
https://hg.mozilla.org/comm-central/rev/322cdc163b997fc08b3f61e87149466eefee22ff#l3.154

Should that be required, I can add the line back in. Overall, there is no drama here. The system behaviour changed slightly and the test results reflect that. Sorry for not running a try before asking for review. I'm still learning about the testing side of things ;-)

Comment 100

[Jorg K (CEST = GMT+2)]

Comment on attachment 8697246 [details] [diff] [review]
Fixing the tests.

OK, try came back green. Who would like to rs+ this?
Sorry about the "r?" SPAM, I only need one review here.

Comment 101

[Magnus Melin [:mkmelin]]

Comment on attachment 8697246 [details] [diff] [review]
Fixing the tests.

Review of attachment 8697246 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. I'll land this shortly

Comment 102

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/e65b9b7f749a

Though now I just realized, you should also clearUserPref of the value in teardownModule() for each test. 
In mozmill the profile survives for the entire directory run (or was that even the whole run)

Comment 103

[Jorg K (CEST = GMT+2)]

Yes, I saw that some tests retrieve preferences, change them and restore them later. I think we can cross that bridge when we come to it and ever want to test something with the paragraph stuff enabled.

Comment 104

[Magnus Melin [:mkmelin]]

Well the reason you should fix it now is that otherwise someone will get hit by different behaviour while testing a single test vs testing the whole suite... and that might be hard to figure out why things didn't work the same.

Comment 105

[aleth [:aleth]]

This needs a comment explaining why it's OK to just turn off the default pref value a user will experience for these tests, as it's not obvious.

I guess the pref affects the test-forward-headers test because now all text content is wrapped in <p> tags, but that is trivial in this case.

Why does flipping this pref affect the charset of a message in plaintext compose mode?

The signature switching test checks that if a signature is changed, no additional blank lines are left behind. But are additional <p> nodes wrapping the sig left behind? What node is the moz-signature class on?

Comment 106

[Jorg K (CEST = GMT+2)]

> you should also clearUserPref of the value in teardownModule().
Coming up.

I'll look at the signature switch and the other ones, too.

Comment 107

[Jorg K (CEST = GMT+2)]

OK, the signature switch is broken in paragraph mode, it grows by one line each time. I'll fix this.

The forward test fails in paragraph mode since a message is created with a given body, when the body is later checked, we expect a text node, but get a <p> node. Understandable.

And the charset test fails twice on
  EXCEPTION: a != b: '' != 'windows-1252'
so the charset of the original message from
  assert_equals(draftMsg.Charset, "windows-1252");
isn't retrieved correctly.

This is totally weird. I can only think that this is a timing issue. New message processing takes a little longer in paragraph mode so the message might not be ready when we go and retrieve the headers. No idea. In any case, the upgrade to UTF-8 works, unless the test gives up on the first failure.

I tested it manually, all works, only the test doesn't like the <p> for some reason to be explored later, perhaps.

Comment 108

[Jorg K (CEST = GMT+2)]

Attachment: Fixing the tests some more.

OK, now resetting the preference.

I still need to look at the signature switch, which grows by one line on each switch.

Comment 109

[Jorg K (CEST = GMT+2)]

Oh, the signature switch fails since it checks the first DOM node of the e-mail and that's a <p> in paragraph mode an not a text.
Line 200: assert_equals(node.nodeValue, "Body, first line.");

 Anyway, need to fix the growing ob one line in paragraph mode.

Comment 110

[aleth [:aleth]]

(In reply to Jorg K (GMT+1) from comment #109)
> Oh, the signature switch fails since it checks the first DOM node of the
> e-mail and that's a <p> in paragraph mode an not a text.
> Line 200: assert_equals(node.nodeValue, "Body, first line.");
> 
>  Anyway, need to fix the growing ob one line in paragraph mode.

A good solution for the test might be to duplicate the failing test function in the test file, and then set the pref to false in one copy of the function and to true in the other. Then you can easily adapt the test to the changed situation, and both ways are tested.

Thanks for checking this.

Comment 111

[aleth [:aleth]]

(In reply to Jorg K (GMT+1) from comment #107)
> OK, the signature switch is broken in paragraph mode, it grows by one line
> each time. I'll fix this.
> 
> The forward test fails in paragraph mode since a message is created with a
> given body, when the body is later checked, we expect a text node, but get a
> <p> node. Understandable.

Yes, for this kind of failure (which is well understood) setting the pref to the old value seems a clean solution.

> And the charset test fails twice on
>   EXCEPTION: a != b: '' != 'windows-1252'
> so the charset of the original message from
>   assert_equals(draftMsg.Charset, "windows-1252");
> isn't retrieved correctly.
> 
> This is totally weird. I can only think that this is a timing issue. New
> message processing takes a little longer in paragraph mode so the message
> might not be ready when we go and retrieve the headers. No idea. In any
> case, the upgrade to UTF-8 works, unless the test gives up on the first
> failure.

This is a bit more concerning. If it's indeed a race condition, the test should wait for whatever it has to wait for.

If you flip the order in which the test functions run, does that affect things?

Comment 112

[aleth [:aleth]]

Comment on attachment 8697409 [details] [diff] [review]
Fixing the tests some more.

Review of attachment 8697409 [details] [diff] [review]:
-----------------------------------------------------------------

Let's land a complete single correct patch to fix the test issues instead of a sequence of partial bits and pieces.

Comment 113

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/491f36c0bf73ca6fa6df7aeb5cad35c7b7388c8f
Backout e65b9b7f749a as it is an incomplete fix for the test failures of bug 330891. rs=aleth DONTBUILD

Comment 114

[Jorg K (CEST = GMT+2)]

Attachment: Fixing the tests.

OK, combined patch to fix the tests with comments as asked for.
I though we want to get a green tree, so we should fix the tests first.

I still need to fix the signature switch in a separate patch, is that OK?

Comment 115

[aleth [:aleth]]

(In reply to Jorg K (GMT+1) from comment #114)
> Created attachment 8697430 [details] [diff] [review]
> Fixing the tests.
> 
> OK, combined patch to fix the tests with comments as asked for.
> I though we want to get a green tree, so we should fix the tests first.

Yes, we want a green tree, but when test failures reveal there are issues that need to be fixed, then it's arguably not wrong that the test fail until that issue is fixed. One can always backout the original patch causing the failure if one is impatient.

From the comments/investigations so far, it seems to me that setting the pref to the old value is a reasonable fix for the failures of test-cloudfile-attachment-urls.js and test-forward-headers.js. 

The failure of test-signature-updating.js revealed a real bug and so it would be nice to cover both pref values in the test (rather than just one) as a fix for the test. But I guess I'll leave that up to mkmelin as the reviewer.

The failure of test-charset-upgrade.js doesn't seem to be understood yet.

Comment 116

[Jorg K (CEST = GMT+2)]

For the record, the signature switch happens here:
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#5388

We start off with
<p>
  <br>
</p>
<br>
<pre> (the signature).

After swapping it once we get:

<p>
  <br>
  <br>
</p>
<pre> (the signature).

After that one more <br> inside the <p> for every swap. To be continued.

Comment 117

[Jorg K (CEST = GMT+2)]

Comment on attachment 8697430 [details] [diff] [review]
Fixing the tests.

I think we should fix the tests now to get a green tree.

If we want to cover more cases, we can add them later, in this case with the signature switch fix.

Backing out the original patch is not really an option since we need it for 45 and the branch date is next Monday.

While being purist is nice, at times one has to be pragmatic. I have seen bugs which land in multiple patches and are marked "leave-open".

Let's see what Magnus says, he already gave r+ to the test fixes, so more complete test fixes should be OK.

It's not that I'm going to walk away from this until it's 100% fixed.

Comment 118

[aleth [:aleth]]

(In reply to Jorg K (GMT+1) from comment #117)
> Backing out the original patch is not really an option since we need it for
> 45 and the branch date is next Monday.
> 
> While being purist is nice, at times one has to be pragmatic. I have seen
> bugs which land in multiple patches and are marked "leave-open".

We're being pragmatic all the time! ;) I didn't back out the original patch because it has strings and we need those landed for 45.

Comment 119

[aleth [:aleth]]

(In reply to Jorg K (GMT+1) from comment #107)
> This is totally weird. I can only think that this is a timing issue. New
> message processing takes a little longer in paragraph mode so the message
> might not be ready when we go and retrieve the headers. No idea. In any
> case, the upgrade to UTF-8 works, unless the test gives up on the first
> failure.

It's indeed strange, I can't reproduce the failure locally whatever the pref value.

Comment 120

[Jorg K (CEST = GMT+2)]

(In reply to aleth [:aleth] from comment #119)
> It's indeed strange, I can't reproduce the failure locally whatever the pref
> value.
Thanks for pursuing it. Let's not be pragmatic and not chase a ghost here given that we're reasonably sure that the paragraph stuff did *not* break the charset upgrade. I couldn't imagine why it would.

Comment 121

[aleth [:aleth]]

Comment on attachment 8697430 [details] [diff] [review]
Fixing the tests.

Review of attachment 8697430 [details] [diff] [review]:
-----------------------------------------------------------------

OK, let's take this and follow up with a better signature test.

Comment 122

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/7ec4f38a2956ee0071e2ed5dc339adbf6dd1bbd6
Bug 330891 - Use editor.CR_creates_new_p=false in certain tests to avoid failures. r=mkmelin,aleth

Comment 123

[Jorg K (CEST = GMT+2)]

To be continued in bug 1231902.

Comment 124

[Axel Grude [:realRaven]]

Quick question for the current release of Thunderbird (38.5.0)

if we have a paragraph like this:

<p style="font: 14pt calibri,sans-serif;">
  Some text...
</p>

What's the current workaround for creating a second paragraph like it? Unfortunately the fonts dropdown just shows "Fixed Width" so we do not really know about the inline styles.

As we cannot create a copy of the <p> is the only way copying the paragraph and then removing the text or is there an easier way of "breaking" the paragraph into 2 using Thunderbird Composer?

Comment 125

[Jorg K (CEST = GMT+2)]

I have no idea. But I'm happy to report that in TB 45, hitting <enter> after the three dots produces:
  <body bgcolor="#FFFFFF" text="#000000">
    <p style="font: 14pt calibri,sans-serif;">Some text... <br>
    </p>
    <p style="font: 14pt calibri,sans-serif;">And more.<br>
    </p>

Comment 126

[Axel Grude [:realRaven]]

(In reply to Jorg K (GMT+1) from comment #125)
> I have no idea. But I'm happy to report that in TB 45, hitting <enter> after
> the three dots produces:
>   <body bgcolor="#FFFFFF" text="#000000">
>     <p style="font: 14pt calibri,sans-serif;">Some text... <br>
>     </p>
>     <p style="font: 14pt calibri,sans-serif;">And more.<br>
>     </p>

\o/

Comment 127

[John Thompson]

The new "paragraph" behaviour in composition is very welcome. Thanks!

One remark though: I find that when using Shift + Enter to insert just a new line, the cursor stays where it is at the end of the current line, instead of descending to the next line as you'd expect. 

However, the new line is in fact inserted, and the characters typed after using Shift + Enter appear on the new line (the cursor moves to the new line on typing the first character).

Trouble is, since the cursor does not move on Shift + Enter, likely many users will press that again. Result: the cursor moves down 1 line on the 2nd Shift + Enter, and one *more* line when the first character of the new line is typed. Not urgent, once one knows the behaviour, but if this can be fixed easily that would be helpful.

Comment 128

[Jorg K (CEST = GMT+2)]

(In reply to John Thompson from comment #127)
> The new "paragraph" behaviour in composition is very welcome. Thanks!
Oh, you like it ;-) We had much criticism about this feature, see bug 1250051 and its duplicates.

> One remark though: I find that when using Shift + Enter to insert just a new
> line, the cursor stays where it is at the end of the current line, instead
> of descending to the next line as you'd expect. 
That's bug 1263357, sadly not easy to fix. In fact, there are more problems in paragraph mode which we're slowly solving. Currently there are two open ones. We're tracking these problems here: bug 1248971.

Comment 129

[John Thompson]

(In reply to Jorg K (PTO during summer, NI me) from comment #128)
> That's bug 1263357, sadly not easy to fix. In fact, there are more problems
> in paragraph mode which we're slowly solving. Currently there are two open
> ones. We're tracking these problems here: bug 1248971.

Thanks for the info. Sorry to hit the wrong thread. I rather feared this might not be easy to fix, looks too simple. Anyway I appreciate the initiative and I appreciate how complicated these seemingly simple things are.

Comment 130

[ThomasH]

And so the argument goes on and on, like in a parliament debating next years budget.  

The simple issue here is that Thunderbird changed its default behavior, and that it annoys users, forces_countless thousands_ of users to spend their time on guessing, experimenting.  Please be so kind to reverse this.  Do not insert any vertical space upon <Enter>.  Simplicity is as always the best way.

At least we know now that <shift><enter> allows to edit the text "as before for years". So I am happy to have discovered that, after have wasted so much time on reversing these strange vertical spaces. I used to press 'automatically' <enter> twice in order to enter an empty line.

Comment 131

[Axel Grude [:realRaven]]

(In reply to ThomasH from comment #130)

> At least we know now that <shift><enter> allows to edit the text "as before
> for years". So I am happy to have discovered that, after have wasted so much
> time on reversing these strange vertical spaces. I used to press
> 'automatically' <enter> twice in order to enter an empty line.

your argument is that the HTML editor should behave like a text only editor.. Why not use plain text email instead? Many rich text editors use <p> tags in order to generate paragraphs. And there is barely any website nowadays that puts text into the <body> tag, because it is not helpful semantically.

You can suppress the extra space of any _paragraph_ by adding a style rule to your HTML template:

<style> 
  p {
    margin-bottom: 0 !important;
    margin-top: 0 !important;
  }
</style>

and then you can make every second paragraph empty. But if you look at your old emails in an HTML editor I bet the vast majority of <br> are in pairs.. Which means you used a lot of unnecessary key strokes, which you don't have to in any modern rich text editor environment. The argument "because I am used to it" should not override the actual _improvement_ of delivering semantically correct HTML with less effort.

Comment 132

[Peter Lairo]

I am surprised there has been no response to the concerns raised in comment #9 (from 2011) and comment #54.

While composing our users will see:

     Dear Sir or Madam

     How are you. Do you like my paragraph style?

     Sincerely
     Thunderbird User

That looks nice. Right?

Now, if we insert a <p> every time the user hits [ENTER], then our recipients will receive this ugly vertically elongated e-mail:

     Dear Sir or Madam


     How are you. Do you like my paragraph style?


     Sincerely

     Thunderbird User

I've received e-mails like this and hated having to look at and read them (with all the excessive scrolling it required). I would really dislike it, if my beloved e-mail client Thunderbird were to create these awful e-mails.

Developers: Is there a way to have your <p> and avoid unknowingly creating ugly e-mails?

Comment 133

[Jorg K (CEST = GMT+2)]

The whole point is that users should not hit enter twice as they did before. To create your nice e-mail you need to enter:

Dear Sir or Madam<enter>

How are you. Do you like my paragraph style?<enter>

Sincerely<shift><enter>
Thunderbird User

I write e-mail like this daily and I find it highly useful to be able to structure my e-mail.

As for the default line-spacing:

As the e-mail body is HTML, TB and other clients will apply appropriate margins, see here for a reference:
https://www.boutell.com/newfaq/creating/paragraphspacing.html

Have we seen any reports that say that paragraphs created with TB look bad in MS Outlook or in the Gmail web interface?

Comment 134

[Richard Leger]

This new default behavior of Thunderbird is completely hill designed!!! Why to make it default, why?why?why? Seriously...

This means every time I write a line and type enter it goes into a new paragraph!!! Completely wrong by design!!!! and usability!!!!!!

Why? Simply because when you type a text, you create new lines <br> much more often than you create new paragraph <p>. The default behavior shall therefore be the opposite. 

Enter = Create a new line, 
Shift+Enter = create a new paragraph
I know this is not the "standard" but let's face it, the so called standard "sucks!"

An even better behavior which would be more intuitive could be to type twice on Enter to get a new paragraph much more user friendly and intuitive as well as logical. Two <br> would be replaced by a <p>.

Makes total sense, isn't it :-)

Comment 135

[Jorg K (CEST = GMT+2)]

This bug is done. If you want to discuss further, please add a comment to bug 1250051.

Richard: There is an option to switch off the new behaviour.
Tools > Options, Composition, General: HTML.
Please read: https://support.mozilla.org/en-US/kb/new-thunderbird-45#w_mail-composition

Comment 136

[Axel Grude [:realRaven]]

(In reply to Richard Leger from comment #134)
> This new default behavior of Thunderbird is completely hill designed!!! Why
> to make it default, why?why?why? Seriously...
> 
> This means every time I write a line and type enter it goes into a new
> paragraph!!! Completely wrong by design!!!! and usability!!!!!!
> 
> Why? Simply because when you type a text, you create new lines <br> much
> more often than you create new paragraph <p>. The default behavior shall
> therefore be the opposite. 
> 
> Enter = Create a new line, 
> Shift+Enter = create a new paragraph
> I know this is not the "standard" but let's face it, the so called standard
> "sucks!"

You don't need to do new lines, they wrap _automatically_. What you really need is something like this:

<style>
  body, p {
    max-width: 40em;
  }
</style>

It might be helpful to put such a rule into your userContent.css. Alternatively you can use a html template that contains such a rule.

It is understandable that you mistakenly used the enter key for "wrapping" but it is definitely problematic as soon as you edit - remember "a computer is not a typewriter". Having said that it would make a whole lot of sense if Composer supplied the user with a "ruler" (like in word) for setting the max-width of the paragraph or even body of the email. I would suggest to raise this as a separate bug.

The point of wrapping is that you can insert / delete words etc without having to re-do a lot of line breaks every time you insert a word.

Comment 137

[Magnus Melin [:mkmelin]]

(In reply to Richard Leger from comment #134)
> Why? Simply because when you type a text, you create new lines <br> much
> more often than you create new paragraph <p>. The default behavior shall
> therefore be the opposite. 

No, sorry but then you've just been doing it all wrong.

Quoting the HTML5 spec: (https://html.spec.whatwg.org/multipage/semantics.html#the-br-element)

  "br elements must be used only for line breaks that are actually part of the content, as in poems or addresses."

  "br elements must not be used for separating thematic groups in a paragraph."

Comment 138

[Richard Leger]

(In reply to Magnus Melin from comment #136)
> (In reply to Richard Leger from comment #134)
> > Why? Simply because when you type a text, you create new lines <br> much
> > more often than you create new paragraph <p>. The default behavior shall
> > therefore be the opposite. 
> 
> No, sorry but then you've just been doing it all wrong.
> 
> Quoting the HTML5 spec:
> (https://html.spec.whatwg.org/multipage/semantics.html#the-br-element)
> 
>   "br elements must be used only for line breaks that are actually part of
> the content, as in poems or addresses."
> 
>   "br elements must not be used for separating thematic groups in a
> paragraph."

I agree with the HTML 5 spec that instead of having two br with should have one p, but in term of usability pressing enter and getting a p is not the right way to do it!!!

My proposal does not go against the HTM5 spec recommendation.

If you press one on Enter key, it create a br, if you press twice (in a reasonable amount of time) or ALT+Enter it would create a p instead of two br. Does it make more sense described this way?

The default previous behaviour shall be return as it was that when editing a paragraph, if I press Enter a new line should be created within the paragraph and not a new paragraph, once again this is insane!!! 

The new ALT+Enter feature could be used to create a new paragraph... which would make sense to me, though once again, pressing twice on the enter key means "I want to create a new paragraph" so the software shall behave accordingly and create a p instead of two br in that case... complying with HTML 5 :-)

Comment 139

[Richard Leger]

(In reply to Axel Grude [:realRaven] from comment #137)
> (In reply to Richard Leger from comment #134)
> > This new default behavior of Thunderbird is completely hill designed!!! Why
> > to make it default, why?why?why? Seriously...
> > 
> > This means every time I write a line and type enter it goes into a new
> > paragraph!!! Completely wrong by design!!!! and usability!!!!!!
> > 
> > Why? Simply because when you type a text, you create new lines <br> much
> > more often than you create new paragraph <p>. The default behavior shall
> > therefore be the opposite. 
> > 
> > Enter = Create a new line, 
> > Shift+Enter = create a new paragraph
> > I know this is not the "standard" but let's face it, the so called standard
> > "sucks!"
> 
> You don't need to do new lines, they wrap _automatically_. 

Sorry!!! I do need my new lines in my paragraphs :-) the way it was...

> What you really
> need is something like this:
> 
> <style>
>   body, p {
>     max-width: 40em;
>   }
> </style>

Sorry again!!! But I don't want this. The text will wrap automatically according to windows size thank you. Though I do want to keep pressing Enter (once again as default behaviour, and I am sure I am not alone) to get a new line within my paragraph when I want to...

> It might be helpful to put such a rule into your userContent.css.
> Alternatively you can use a html template that contains such a rule.

Sorry but you miss the point. I am not talking about only myself... and this not how Thunderbird shall behave by default, obliging me to twick .css file to return a default feature that was already existing for ages, if ain't broken why to fix it!!! 
:-)

> It is understandable that you mistakenly used the enter key for "wrapping"
> but it is definitely problematic as soon as you edit - remember "a computer
> is not a typewriter". 

Computer text editor interface have been design based on typewriter features... and running this way well for years :-)

> Having said that it would make a whole lot of sense if
> Composer supplied the user with a "ruler" (like in word) for setting the
> max-width of the paragraph or even body of the email. I would suggest to
> raise this as a separate bug.

Don't need Thunderbird composer to behave like a Word processor sorry...

> The point of wrapping is that you can insert / delete words etc without
> having to re-do a lot of line breaks every time you insert a word.

I agree with the wrapping feature in creating paragraph when adequate and requested by end-users facilitating life... but not the way it is currently newly implemented in Thunderbird... Now if I forward or reply to email some of the text become separate paragraph including every line of my signature that I want to keep in the same paragraph :-) Will probably file a bug about this one soon... once I figure the exact circumstance it happens...

Once again, I am not disagreeing with the goal to replace two br with one p, but the way it is currently implemented by default still sucks to achieve it.

I know the option exist to turn it off, but still I would prefer to have the previous default behaviour back, with new feature set as opt-in option.

Hoping I will not reach dead ears :-)

Comment 140

[John Thompson]

(In reply to Richard Leger from comment #138)
If we were designing the first ever text editor I'm not saying the idea of pressing Enter once for a new line and twice for a new para is a bad idea.

[Hé hé just pressed Enter twice ... ;-))]
BUT we're not. We're in a world where the norm (in word processing) is that 1 press on Enter creates a paragraph. Most offer Shift+Enter for a new line. This bug, as you can see from its title, aims to implement this long-standing norm in TB.

Comment 141

[Axel Grude [:realRaven]]

(In reply to Richard Leger from comment #139)
> (In reply to Axel Grude [:realRaven] from comment #137)
> > (In reply to Richard Leger from comment #134)
> > > This new default behavior of Thunderbird is completely hill designed!!! Why
> > > to make it default, why?why?why? Seriously...
> > > 
> > > This means every time I write a line and type enter it goes into a new
> > > paragraph!!! Completely wrong by design!!!! and usability!!!!!!
> > > 
> > > Why? Simply because when you type a text, you create new lines <br> much
> > > more often than you create new paragraph <p>. The default behavior shall
> > > therefore be the opposite. 
> > > 
> > > Enter = Create a new line, 
> > > Shift+Enter = create a new paragraph
> > > I know this is not the "standard" but let's face it, the so called standard
> > > "sucks!"
> > 
> > You don't need to do new lines, they wrap _automatically_. 
> 
> Sorry!!! I do need my new lines in my paragraphs :-) the way it was...
> 
> > What you really
> > need is something like this:
> > 
> > <style>
> >   body, p {
> >     max-width: 40em;
> >   }
> > </style>
> 
> Sorry again!!! But I don't want this. The text will wrap automatically
> according to windows size thank you. 

Ok, factually what you are doing is forced manual wrapping, but you know that the text automatically wraps by window size. You will see that if your emails are replied to and quoted multiple time, your manual wraps may generate orphaned words on the next line, which is one strong argument against abusing <br> for word wrapping. The fact that you know about Thunderbird's "window-width" wrapping (which is essentially setting 

* {width:100%;} 

and still do manual wrapping shows me that there is something that needs to be fixed fundamentally to conform to a natural, more readablee wrapping behavior; and I would agree that in the age of wide monitors going for 100% of the window width is definitely ergonomically broken.

> Though I do want to keep pressing Enter
> (once again as default behaviour, and I am sure I am not alone) to get a new
> line within my paragraph when I want to...

what do you do if you insert or delete half a sentence at the beginning of your paragraph? because you seem to be treating the HTML composer like a text editor (notepad). And with that (unless you use word wrap) you have a whole load of unneccesary fixing to do when you edit with manual wrapping. That is essentially what the concept of paragraphs fixes: these are containers that among other things take care of wrapping the text within a semantically conjunct area.

> 
> > It might be helpful to put such a rule into your userContent.css.
> > Alternatively you can use a html template that contains such a rule.
> 
> Sorry but you miss the point. I am not talking about only myself... and this
> not how Thunderbird shall behave by default, obliging me to twick .css file
> to return a default feature that was already existing for ages, if ain't
> broken why to fix it!!

I am not sure that you know you are in the majority here; it would be interesting to see how many people use <br> for wrapping their text. If the majority do this, then for me this is an even stronger argument for providing a visible ruler / outline while composing so they know they do not need to treat the HTML editor like a typewriter.
 
> > It is understandable that you mistakenly used the enter key for "wrapping"
> > but it is definitely problematic as soon as you edit - remember "a computer
> > is not a typewriter". 
> 
> Computer text editor interface have been design based on typewriter
> features... and running this way well for years :-)

This is true for text editors. HTML editors are aiming a little higher, as they (just like word processors) are aiming to provide a simple UI for achieving readable layout. I have discussed on the Thunderbird planning list before on how Composer is lacking additional UI features (e.g. for user defined styles) in order to become a more useful and ergonomic tool (just like word processors did over the years they have been developed).

I still am asking myself why you aren't using plain text mode, it seems to be much closer to what you want? Manual wrapping is a mistake; go into your internet history and try to find me just one page that uses <br> for wrapping paragraphs; I bet you will have a really difficult time finding even just one single example. 

It is true that we would use manual wrapping in a <pre> tag, because that is essentially designed to give back a typewriter mode.

> 
> > Having said that it would make a whole lot of sense if
> > Composer supplied the user with a "ruler" (like in word) for setting the
> > max-width of the paragraph or even body of the email. I would suggest to
> > raise this as a separate bug.
> 
> Don't need Thunderbird composer to behave like a Word processor sorry...

Yes that's you. So I suggest:

Go to Thunderbird options
Click on Composition
Select the General Tab
Uncheck the option "When using paragraph format. the enter key creates a new paragraph"

> I know the option exist to turn it off, but still I would prefer to have the
> previous default behaviour back, with new feature set as opt-in option.

the point is that it works for the majority of users and a vocal minority complains about this change. I will prove this to you with a "circumstantial" attachment of 2 emails my 75 year old mum has sent me. She is using the new feature without even knowing that it is there and her emails are much better readable now.

> Now if I forward or reply to email some
> of the text become separate paragraph including every line of my signature
> that I want to keep in the same paragraph :-) Will probably file a bug about
> this one soon... once I figure the exact circumstance it happens...

please do, and add a screen shot because it is hard to visualize.

Comment 142

[Axel Grude [:realRaven]]

Attachment: 2 Example Emails

This is a real world example of a 75 year old user (my mum). I tried to explain to her before (before bug 330891 landed) that she needed to press enter twice in order to make her paragraphs readable, which she never did; intuitively she accepted automatic wrapping and used the enter key for ending her paragraphs. This leads to a poorly readable email (top example), caused by a bad / non-intuitive behavior of Composer. Sure the user can adapt by learning to hit Enter two times, but this means she has to change her behavior.

She was using word processors for a longer time than email, which is partly due to her age. But she does understand the concept of being able to layout her writing with paragraphs and the basic concept of automatic wrapping, and she did not need the new "Enter = splits <p>" feature explained to her. 

I know this is only circumstantial evidence for the benefits of this feature but it should have the same weight as your argument which may claim that the majority of users benefits from the old behavior because they prefer to wrap their emails manually. 

I just wanted to highlight that the feature wasn't assumed to be as disruptive by a non-technical person as you make it out to be; and it improved my email live (as somebody who receives and has to read these emails) without any additional interaction - I did not have to explain it to her, the emails are just magically easier to read.

So I am not trying to convince you here, it is just that to *users who accept automatic wrapping* their live is made more convenient by having to hit Enter less often, so they have to do *less keystrokes* to achieve a readable result. Sure they will have to adapt their typing behavior but overall it is a ergonomic improvement. 

If I assume that users who wrap manually are in the minority then the new behavior is definitely superior.

Comment 143

[Jorg K (CEST = GMT+2)]

<off-topic>
Are you still arguing here? ;-)
I was pretty much against this feature when I implemented it. Now I can't live without it. If enter doesn't create a new paragraph because I somehow got into "body text" mode, I stop and correct that first. Sure, here in Bugzilla, the text entry box is like a type-writer, so enter takes you to the next line. But HTML e-mail composition is a different kettle of fish. Those who want plaint text composition should enable that.

Now, as much as you argue here, I don't think we will reverse the decision. However, we realised that the label for the option was a little confusing, so in TB 52 this will read:
[ ] Use Paragraph format instead of Body Text by default
instead of
[ ] When using paragraph format, the enter key creates a new paragraph.
We changed that in bug 1273841.
</off-topic>

Comment 144

[Richard Leger]

(In reply to John Thompson from comment #140)
> (In reply to Richard Leger from comment #138)
> If we were designing the first ever text editor I'm not saying the idea of
> pressing Enter once for a new line and twice for a new para is a bad idea.

So why not to implement it in TB in addition of the current default. It could become the new "Norm" or even an international standard that other text editor and word processing software could use.
It could be activated if I opt-out of the default option.

> 
> [Hé hé just pressed Enter twice ... ;-))]
> BUT we're not. We're in a world where the norm (in word processing) is that
> 1 press on Enter creates a paragraph. Most offer Shift+Enter for a new line.
> This bug, as you can see from its title, aims to implement this
> long-standing norm in TB.

That is misleading. Once should not forget that this is not a "norm" this is a feature that was imposed by a proprietary software company not so long ago, and has since only very recently been picked up (wrongly to my opinion) by others...

Comment 145

[Richard Leger]

(In reply to Axel Grude [:realRaven] from comment #141)
> (In reply to Richard Leger from comment #139)
> > (In reply to Axel Grude [:realRaven] from comment #137)
> > > (In reply to Richard Leger from comment #134)
> > > > This new default behavior of Thunderbird is completely hill designed!!! Why
> > > > to make it default, why?why?why? Seriously...
> > > > 
> > > > This means every time I write a line and type enter it goes into a new
> > > > paragraph!!! Completely wrong by design!!!! and usability!!!!!!
> > > > 
> > > > Why? Simply because when you type a text, you create new lines <br> much
> > > > more often than you create new paragraph <p>. The default behavior shall
> > > > therefore be the opposite. 
> > > > 
> > > > Enter = Create a new line, 
> > > > Shift+Enter = create a new paragraph
> > > > I know this is not the "standard" but let's face it, the so called standard
> > > > "sucks!"
> > > 
> > > You don't need to do new lines, they wrap _automatically_. 
> > 
> > Sorry!!! I do need my new lines in my paragraphs :-) the way it was...
> > 
> > > What you really
> > > need is something like this:
> > > 
> > > <style>
> > >   body, p {
> > >     max-width: 40em;
> > >   }
> > > </style>
> > 
> > Sorry again!!! But I don't want this. The text will wrap automatically
> > according to windows size thank you. 
> 
> Ok, factually what you are doing is forced manual wrapping, but you know
> that the text automatically wraps by window size. You will see that if your
> emails are replied to and quoted multiple time, your manual wraps may
> generate orphaned words on the next line, which is one strong argument
> against abusing <br> for word wrapping. The fact that you know about
> Thunderbird's "window-width" wrapping (which is essentially setting 
> 
> * {width:100%;} 
> 
> and still do manual wrapping shows me that there is something that needs to
> be fixed fundamentally to conform to a natural, more readablee wrapping
> behavior; and I would agree that in the age of wide monitors going for 100%
> of the window width is definitely ergonomically broken.

Automatic wrapping is not the same thing as a line return within a paragraph.
And one shall not be used to replace the other...

> > Though I do want to keep pressing Enter
> > (once again as default behaviour, and I am sure I am not alone) to get a new
> > line within my paragraph when I want to...
> 
> what do you do if you insert or delete half a sentence at the beginning of
> your paragraph? because you seem to be treating the HTML composer like a
> text editor (notepad). 

An HTML editor IS a text editor!!! It just provide few additional feature to format text... but it IS a text editor. With the new set default option you now create a discrepancy between how a text editor behave and an HTML editor behave in its basic functionality (typing text).

> And with that (unless you use word wrap) you have a
> whole load of unneccesary fixing to do when you edit with manual wrapping.
> That is essentially what the concept of paragraphs fixes: these are
> containers that among other things take care of wrapping the text within a
> semantically conjunct area.

I am not using Enter to "wrap" the text (this is done automatically), I am using Enter to create a new line within a paragraph to make an em-phase, create a sub-paragraph within the paragraph.

> > 
> > > It might be helpful to put such a rule into your userContent.css.
> > > Alternatively you can use a html template that contains such a rule.
> > 
> > Sorry but you miss the point. I am not talking about only myself... and this
> > not how Thunderbird shall behave by default, obliging me to twick .css file
> > to return a default feature that was already existing for ages, if ain't
> > broken why to fix it!!
> 
> I am not sure that you know you are in the majority here; it would be
> interesting to see how many people use <br> for wrapping their text. If the
> majority do this, then for me this is an even stronger argument for
> providing a visible ruler / outline while composing so they know they do not
> need to treat the HTML editor like a typewriter.

I am not using <br> to wrap the text, and if you consider people do so, I agree it is wrong.
Text wrap automatically but within a paragraph, you may want to create sub-paragraph without creating two paragraph.

> > > It is understandable that you mistakenly used the enter key for "wrapping"
> > > but it is definitely problematic as soon as you edit - remember "a computer
> > > is not a typewriter". 
> > 
> > Computer text editor interface have been design based on typewriter
> > features... and running this way well for years :-)
> 
> This is true for text editors. HTML editors are aiming a little higher, as
> they (just like word processors) are aiming to provide a simple UI for
> achieving readable layout. I have discussed on the Thunderbird planning list
> before on how Composer is lacking additional UI features (e.g. for user
> defined styles) in order to become a more useful and ergonomic tool (just
> like word processors did over the years they have been developed).

An HTML editor is essentially a text editor. It offers additional feature to format text, but it IS a text editor, I agree with additional features.
I am not agains your goal to achieve simplicity and readable layout, on the contrary I believe my proposal was reaching that goal in a much simpler and intuitive manner, without breaking compatibilty between a simple text editor (notepad, TB text editor) and an HTML editor (TB Composer).

> I still am asking myself why you aren't using plain text mode, it seems to
> be much closer to what you want? 

Nope. I want to use the HTML editor in an intuitive manner. I do use advance feature to format text :-)
I do like the idea to help formatting proper HTML in TB Composer, but I just regret the way it is currently implemented by default. At least would you consider the possibility to add feature so pressing twice Enter would create a p instead of two br if opt-out of the current default in Options?

> Manual wrapping is a mistake; go into your
> internet history and try to find me just one page that uses <br> for
> wrapping paragraphs; I bet you will have a really difficult time finding
> even just one single example. 
> 
> It is true that we would use manual wrapping in a <pre> tag, because that is
> essentially designed to give back a typewriter mode.

Once again, I think you are mistaking my point. I am not pressing Enter to wrap/format text with a br, I use Enter to structure my text into creating sub-paragraph within a paragraph. For the record I do use the auto-wraping feature in TB (that already exist).

> > 
> > > Having said that it would make a whole lot of sense if
> > > Composer supplied the user with a "ruler" (like in word) for setting the
> > > max-width of the paragraph or even body of the email. I would suggest to
> > > raise this as a separate bug.
> > 
> > Don't need Thunderbird composer to behave like a Word processor sorry...
> 
> Yes that's you. So I suggest:
> 
> Go to Thunderbird options
> Click on Composition
> Select the General Tab
> Uncheck the option "When using paragraph format. the enter key creates a new
> paragraph"

I know I can do that thanks, and I have no power to change your mind. But still this default behaviour sucks...

> > I know the option exist to turn it off, but still I would prefer to have the
> > previous default behaviour back, with new feature set as opt-in option.
> 
> the point is that it works for the majority of users and a vocal minority
> complains about this change. I will prove this to you with a
> "circumstantial" attachment of 2 emails my 75 year old mum has sent me. She
> is using the new feature without even knowing that it is there and her
> emails are much better readable now.

I am convince your 75 year old mum would be quite satisfied as well to know that pressing twice on Enter she can create paragraph as well which would keep her emails much better readable.

> > Now if I forward or reply to email some
> > of the text become separate paragraph including every line of my signature
> > that I want to keep in the same paragraph :-) Will probably file a bug about
> > this one soon... once I figure the exact circumstance it happens...
> 
> please do, and add a screen shot because it is hard to visualize.

I will. Should I create a separate bug or add it here?

Comment 146

[Richard Leger]

(In reply to Axel Grude [:realRaven] from comment #142)
> Created attachment 8757627 [details]
> 2 Example Emails
> 
> This is a real world example of a 75 year old user (my mum). I tried to
> explain to her before (before bug 330891 landed) that she needed to press
> enter twice in order to make her paragraphs readable, which she never did;

But she could if she wanted to. User power of choice.

> intuitively she accepted automatic wrapping and used the enter key for
> ending her paragraphs. This leads to a poorly readable email (top example),
> caused by a bad / non-intuitive behavior of Composer. Sure the user can
> adapt by learning to hit Enter two times, but this means she has to change
> her behavior.

Don't tell me that pressing twice on Enter is not intuitive and self understandable to create a paragraph!!! My mother is 70 and she has no problem to understand this concept. If you end-user don't want to do it, it should be their choice not the software one.

> She was using word processors for a longer time than email, which is partly
> due to her age. But she does understand the concept of being able to layout
> her writing with paragraphs and the basic concept of automatic wrapping, and
> she did not need the new "Enter = splits <p>" feature explained to her. 

The pressing twice on Enter to create a p does not need to be explained either, it is WYSIWYG!
Therefore is natural...
 
> I know this is only circumstantial evidence for the benefits of this feature
> but it should have the same weight as your argument which may claim that the
> majority of users benefits from the old behaviour because they prefer to wrap
> their emails manually. 

I am not asking for old behaviour, don't get me wrong. I am asking to keep the goal you have, which is a great evolution that I support, but not currently the way it is implemented. Does that make sense?
I am trying to argue on a different default implementation that does not brake compatibility with existing feature and still achieve your goal.
I am hoping to get at least my voice heard if not to revert the default feature, to at least implement it in an alternative way for those that may want to opt-out from the current default behaviour (which should be considered bad)...

> I just wanted to highlight that the feature wasn't assumed to be as
> disruptive by a non-technical person as you make it out to be; and it
> improved my email live (as somebody who receives and has to read these
> emails) without any additional interaction - I did not have to explain it to
> her, the emails are just magically easier to read.

My proposal achieve the same goal, while still leaving liberty to end-user to choose, while keeping compatibility with existing behaviour (prior the default changed).
TB Composer window is an advance text editor (HTML) but shall not be considered as a word processor such as Word :-)

> So I am not trying to convince you here, it is just that to *users who
> accept automatic wrapping* their live is made more convenient by having to
> hit Enter less often, so they have to do *less keystrokes* to achieve a
> readable result. Sure they will have to adapt their typing behavior but
> overall it is a ergonomic improvement. 

Automatic wrapping was already existing prior the Enter key default behaviour was changed.
And I agree it is a convenient feature that do let to less Enter keystroke :-) It does help reading.
But pressing Enter and getting a line break has been the default behaviour for years (a century even...almost!). Just the phrase "Sure they will have to adapt their typing behavior" is wrong thinking... positive change shall not force adaptive behaviour!
In overall, I do dis-aprove the current default implemented feature as being an ergonomic improvement.

But I approve the idea of facilitating the creation of proper HTML paragraph for better readability, just not current implementation of it.

> If I assume that users who wrap manually are in the minority then the new
> behavior is definitely superior.

Pressing Enter shall not be seen as a manual wrap but as an intention to structure the text, and by structure I don't mean display formatting... hope that make sense...

Comment 147

[:aceman]

No, it depends on how you define a "text editor". But going by the more or less used definition, any WYSIWYG HTML composer is more a word processor.
https://en.wikipedia.org/wiki/Text_editor#Plain_text_files_vs._word_processor_files
And in such there is more context to the decision how to wrap lines or paragraphs, not just the user pressing Enter. You forget that to <cite>just provide few additional feature to format text</cite> must take actions that make it no longer be a stupid text editor.

If you want the editor to perfectly observe all your Enter presses, you need a simple text editor. The you can use the plain text editor in TB (well set the number of columns to 999 so it does not wrap automaticall), or maybe try some Tb extension that allows you to edit the HTML source of the composed message. There you can produce whatever output you need.