1193153 - Copy/paste from a plain text editor loses white-space (multiple spaces/blanks, tabs, newlines) since plain text editor uses "white-space: pre-wrap" - Caused by bug 1174452

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Rainer Bielefeld]

Steps how to reproduce with SeaMonkey en-US 2.39a1 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0 from official download area)  Gecko/20100101  Firefox/42.0  Build 20150806070100  (Classic Theme) on German WIN7 64bit:

1. In MailNews client click inbox of a POP3 account with default
   “compose messages as plain text”.
2. Click 'Compose ( → compose in Plain Text)'
  » Mail Composer windows appears
3. Copy/paste 4 lines starting with a dot and trailing empty line
   from “Test paragraphs” below to
   Mail composer
  » Looks like below
4. ' <control+a> → rightclick → copy'
5. 'click into trailing empty line → rightclick → paste
   Expected: pasted lines identical to source
   Actual (bug): all lines with dots have the same length, only 1 single
                 space between the dots



--- Test paragraphs ---
. . . . . . . . 1 Space
.  .  .  .  .  .  .  . 2 spaces
.   .   .   .   .   .   .   . 3 spaces

--- Test paragraphs ---

Additional information
a) Under conditions I currently do not understand in mail source code
   I find all repeated space characters replaced by unbreakable spaces
   “&nbsp;” (Web pages Composer seems to do that always), but that 
    currently is not reproducible for me in e-mail composer and I do not 
   know 
   whether this might be related
b) Web Page composer is not affected, copy/paste due to steps 1 … 5 
   leads to an identical copy
c) This problem has been observed by user Gerd Steffens at
   de.comm.software.mozilla.nightly-builds in thread 
   “Composer - Tabs geändert?”

Comment 1

[Rainer Bielefeld]

d) OS = ALL due to confirmation for Linux by user Hartmut Figge at
   de.comm.software.mozilla.nightly-builds in thread 
   “Composer - Tabs geändert?”
e) Also REPRODUCIBLE with EN-US SeaMonkey 2.35(γ)  (Windows NT 6.1; WOW64; 
   rv:38.0)  Gecko/20100101 Build 20150807220800 (Classic Theme) 
   on German WIN7 64bit
f) by user Hartmut Figge at
   de.comm.software.mozilla.nightly-builds in thread 
   “Composer - Tabs geändert?”:
   Last good: 2015-04-09 15:26:00 PDT   c-c:a0cba0d60fcd m-c:dd32e3ff3717
   First bad: 2015-04-15 09:19:00 PDT   c-c:fe7caa9e9426 m-c:379a89a9fb95
d) in <https://bugzilla.mozilla.org/buglist.cgi?cmdtype=dorem&remaction=run&namedcmd=DUPs1193153&sharer_id=41036>
   I found possible DUP "Bug 1160880 - Selecting text & replying in plain text
   eats spaces in some situations"
e) New due to confirmatin by user Hartmut Figge

Comment 2

[Rainer Bielefeld]

f) Also reproducible with TB 42.0a1 (2015-08-09)
g) copy/past is THE feature in editing, so MAJOR
h) Also might be related: "Bug 1167926 - Copy-pasted text with moz-txt-link-abbreviated link omits preceding space, breaks mailinglist-confirmation"

Comment 3

[Rainer Bielefeld]

d): appearance of problems in Bug 1160880 and Bug 1167926 match with
    Last "goood / first bad" here and also are concerning something with
    space character, but currently I am not sure whether they really are DUPs
    (special cases of this one); currently I doubt.

Comment 4

[Wayne Mery (:wsmwk)]

Can you determine the regression range?

Comment 5

[Rainer Bielefeld]

(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #4)

Not more exactly than comment 1 f) by Hartmut Figge

Comment 6

[Wayne Mery (:wsmwk)]

Ideally we would have a one day regression range, i.e. works on date N, fails on date N+1

Comment 7

[Hartmut Figge]

(In reply to Wayne Mery (:wsmwk, use Needinfo for questions) from comment #6)
> Ideally we would have a one day regression range, i.e. works on date N,
> fails on date N+1

Of course. Difficult to obtain though in a period where a build is not possible due to bustages. ;)

Comment 8

[Jorg K (CEST = GMT+2)]

I think you're busily reporting duplicates of bug 1174452.

Your bug here is in the wrong product, this is not a TB bug, it's a Mozilla Core editor problem.

The regression was caused by:
https://hg.mozilla.org/mozilla-central/rev/47d62ded4e5f
on 10th April 2015.

As you've said in comment #1:
Last good: 2015-04-09 15:26:00 PDT
First bad: 2015-04-15 09:19:00 PDT

Comment 9

[Andreas Schamanek]

@JorgK Can you elaborate why this is a dup of bug 1174452? I thought 1174452 was about HTML, this here is about plain text. However, you might be right that both bugs are caused by the same regression.

In any case, for Thunderbird 38.1 (Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0) I could verify today that this is very likely a duplicate of bug 1187476.

Comment 10

[Jorg K (CEST = GMT+2)]

It's a common misunderstanding that there is something like a plain text editor. There is not. It's all HTML (with some stuff hidden). You can simulate all TB editing in a <div contenteditable> in Firefox.

Also, if it's a M-C core problem, which this is, you need to reproduce the problem in Firefox, since the M-C people are not interested in Thunderbird. This is what I've done in the other bug.

Comment 11

[Jorg K (CEST = GMT+2)]

FYI: The TB "plain text" mode is nothing but a HTML editor where everything has this CSS style:
  font-family: -moz-fixed; white-space: pre-wrap; width: 72ch;

Comment 12

[Wayne Mery (:wsmwk)]

I think we can better track the open issue (and users will recognize it better when searching bugzilla) if instead we make this blocked by bug 1174452, even though it is indeed the same bug

Comment 13

[Jorg K (CEST = GMT+2)]

OK, we can use this bug to land the M-C patch on the TB ESR 38 release branch.

Comment 14

[WADA:World Anti-bad-Duping Agency]

Closing as dup of bug 1174452. If duping is wrong, please re-open.

Comment 15

[Jorg K (CEST = GMT+2)]

This was meant to be left open as per comment #12.

Comment 20

[WADA:World Anti-bad-Duping Agency]

Added "white-space: pre;" in bug summary for ease of search and understanding bug.

Comment 21

[Jorg K (CEST = GMT+2)]

Can we please leave the summary alone now. As explained in comment #11: A so-called "plain text editor" is just a normal editor using CSS "white-space: pre-wrap; width: 72ch;".

Comment 22

[Jorg K (CEST = GMT+2)]

I talked to Ehsan and he told me that bug 1174452 is too hard to fix for me.

So the only option to make this work would be to back out the changeset which caused the regression on the TB 38 release branch:
https://hg.mozilla.org/mozilla-central/rev/47d62ded4e5f

Kent, what do you think?

Comment 23

[Kent James (:rkent)]

What is the plan for m-c with respect to that patch? Changes on THUNDERBIRD_38_VERBRANCH have so far only been pushing changes already landed on m-c into m-esr38 a little faster than the FF folk were comfortable with.

I'm mostly relying on your judgement, Jorg, of the correct things to do in editor. But trying to maintain a permanent difference between m-c and whatever Thunderbird compiles with is a major step that would require a pretty serious issue.

If Thunderbird is much more affected than FF we could consider changes in TB 38 but only if there is an understanding of the long-term plan to get us resynced with m-c.

Comment 24

[Jorg K (CEST = GMT+2)]

(In reply to Kent James (:rkent) from comment #23)
> What is the plan for m-c with respect to that patch? Changes on
> THUNDERBIRD_38_VERBRANCH have so far only been pushing changes already
> landed on m-c into m-esr38 a little faster than the FF folk were comfortable
> with.
Understood.

> I'm mostly relying on your judgement, Jorg, of the correct things to do in
> editor. But trying to maintain a permanent difference between m-c and
> whatever Thunderbird compiles with is a major step that would require a
> pretty serious issue.
I believe that many people who use "plain text" e-mail (internally "white-space: pre-wrap; width: 72ch;") are affected. There are four duplicates.

> If Thunderbird is much more affected than FF we could consider changes in TB
> 38 but only if there is an understanding of the long-term plan to get us
> resynced with m-c.
Yes, TB is more affected than FF. However, there is no medium-term plan to fix this in M-C. Once M-C fixes it, we would resync. Landing this on the TB 38 release branch is simple, it's more interesting what we do, when TB 45 comes around. Do we do another TB 45 release branch? We did for TB 38, since we wanted to land editor changes early. For TB 45 it might not be necessary.

So maybe we ask Wayne. I think he has got something to say about this bug. Is this important enough to land something special for TB which will not get resynced with M-C in the foreseeable future?

Comment 25

[Jorg K (CEST = GMT+2)]

Ignore what I said altogether. Backing out 
https://hg.mozilla.org/mozilla-central/rev/47d62ded4e5f
would have no effect as it would put code back into a spot which is not compiled for TB (#ifndef MOZ_THUNDERBIRD):
http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDocumentEncoder.cpp#1434
This was put in there in bug 1125956 and there is also a follow-up to fix it properly: bug 1141786.

Comment 26

[Jorg K (CEST = GMT+2)]

OK, I created a new bug to implement a Thunderbird specific hack: Bug 1214377.

I detected that in the exact spot that needed to be touched, there was already Thunderbirds specific code with a #ifndef MOZ_THUNDERBIRD added by Ehsan. So I am lobbying making the conditional compilation work better until a real solution is implemented.

Yes, it's a hack, but the hack was already there, so why not change it to be useful.

Comment 27

[Kent James (:rkent)]

This will be solved by bug 1214377 right? If so, it should probably be a dup.

Comment 28

[Jorg K (CEST = GMT+2)]

Right for the first part of your statement.

Comment #8 made it a duplicate of bug 1174452.
Comment #12 undid that.
Comment #14 made it a duplicate of bug 1174452.
After comment #15 this was undone again.
Since there is no fix for bug 1174452 in sight, I created a hack in bug 1214377 (which will land any minute now).

To land some M-C change on the TB 38 release branch we need a bug à la "Take bug so-and-so to TB 38.x". This is what this bug is useful for, see comment #13. You agree?

Comment 29

[Kent James (:rkent)]

OK, so this bug will morph into a request to take a patch in our branch. Yes it fine to leave open for that.

Comment 30

[Jorg K (CEST = GMT+2)]

Fixed by bug 1214377: https://hg.mozilla.org/mozilla-central/rev/b3d28572da60

Comment 31

[Jorg K (CEST = GMT+2)]

Actually:
Fixed by bug 1214377: https://hg.mozilla.org/mozilla-central/rev/42627d5369b3

Comment 33

[Jorg K (CEST = GMT+2)]

Since this has landed recently on M-C trunk, I did some testing on a TB daily.
I can confirm that this bug is fixed and that bug 1125956 hasn't regressed.
So until the M-C people shake it up again, we are cool.

Comment 34

[Jorg K (CEST = GMT+2)]

Damn, I've been using TB 45 (daily) in "production" now and it looks like bug 1125956 *HAS* indeed regressed. Looks like I got the M-C patch wrong. I'll look into it further. For the time being, there is nothing to land in TB 38.x. Sorry!

Comment 35

[Jorg K (CEST = GMT+2)]

OK, finally we have this fixed on TB 45 (daily).
The first attempt was
https://hg.mozilla.org/mozilla-central/rev/42627d5369b3 (comment #31, bug 1214377 comment #12)
That wasn't quite right and has now been corrected:
https://hg.mozilla.org/mozilla-central/rev/c9bc8f46ac3e (bug 1214377 comment #25)
You need to land both patches, since the second one corrects the first one.

This could be uplifted to the TB38 release branch.

Comment 36

[Jorg K (CEST = GMT+2)]

Fixed by bug 1214377.

Comment 38

[Rainer Bielefeld]

Still REPRODUCIBLE with inofficial (from <http://seamonkey.callek.net/contrib/>)  en-US SeaMonkey 2.44a1  Mozilla/5.0 (Windows NT 6.1; x64; rv:47.0)  Gecko/20100101 Firefox/47.0 Build 20160208090255  (Default Classic Theme)  on German WIN7 64bit - what ever that might mean.

Comment 39

[Jorg K (CEST = GMT+2)]

I'm not sure what you're doing. All works for me on TB 45 beta, since my fix landed on Gecko 45, see comment #35. I've also tried current trunk, TB 47. SM 2.44 is build with Gecko 47 so it should work.

The following works for me:

Copy the following six lines from here (Bugzilla) into a plaintext e-mail:
--- Test lines ---
1) . . . . . . . . 1 Space
2) .  .  .  .  .  .  .  . 2 spaces
3) .   .   .   .   .   .   .   . 3 spaces

--- Test line ---

In the plaintext e-mail select lines 1) 2) and 3) and copy them to the empty line following line 3), or add more lines at the end and copy them there. You can also "select all" <ctrl a>, then copy and paste.

In the plaintext e-mail select lines 1) 2) and 3) and copy them to Notepad.

All works!

Comment 40

[Hartmut Figge]

Attachment: Test with my current SM-Trunk

Here is the result of testing my current SM-Trunk, built with GTK2.
User agent: Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/2016020900 SeaMonkey/2.44a1-h

After copying the first three lines into the compose window, these three lines were selected together and then pasted below the text.

Comment 41

[Peter Langston]

Jorg K,
    Your test is a straw man.  Of course it works since each line begins with a non-space character.
Try this real test...
Enter these three (non-blank) text lines into a plaintext message:

Dear Correspondent,
     I am indenting this first line of my paragraph
because that's how I want my paragraph to be formatted.

and then copy them and paste them in again.  Anytime I do that I get:

Dear Correspondent,
I am indenting this first line of my paragraph
because that's how I want my paragraph to be formatted.

That is a failure.  Copy/paste from a plain text editor loses white-space.

The status for this bug should be: unresolved and unfixed.

I am running Thunderbird 38.5.1 which I'm told is the release channel version.

-Peter Langston

Comment 42

[Jorg K (CEST = GMT+2)]

(In reply to Hartmut Figge from comment #40)
> Here is the result of testing my current SM-Trunk, built with GTK2.
Sorry, of course. There is conditional compilation at work, the fix is only compiled for Thunderbird!
https://dxr.mozilla.org/comm-central/source/mozilla/dom/base/nsDocumentEncoder.cpp#1443

(In reply to Peter Langston from comment #41)
> Try this real test...
This works as well.

> The status for this bug should be: unresolved and unfixed.
No. This bug is fixed for Thunderbird from version 45.

> I am running Thunderbird 38.5.1 which I'm told is the release channel
> version.
This bug is fixed in Thunderbird 45 and later. TB 45 will be in the release channel from mid-March. You can try a beta version now: https://www.mozilla.org/en-US/thunderbird/channel/

Comment 43

[Mason]

It would be nice to fix the bug for Seamonkey too.

Bug 1230163 could be used to track the progress of porting the fix to Seamonkey.

@Jorg, the #ifdef MOZ_THUNDERBIRD is to avoid changing the behavior of the function in Firefox?
(Is mozilla/dom/base/nsDocumentEncoder.cpp shared between Firefox and Thunderbird?)

In that case, how to solve the problem for Seamonkey (which has messenger + browser)?
It would need to be a run-time decision?

Comment 44

[Hartmut Figge]

(In reply to Jorg K (GMT+1) from comment #42)
> (In reply to Hartmut Figge from comment #40)
> > Here is the result of testing my current SM-Trunk, built with GTK2.
> Sorry, of course. There is conditional compilation at work, the fix is only
> compiled for Thunderbird!
> https://dxr.mozilla.org/comm-central/source/mozilla/dom/base/
> nsDocumentEncoder.cpp#1443

:-)
Now I'm using a patch which removes the conditional and it works in my SM.

Comment 45

[Jorg K (CEST = GMT+2)]

(In reply to Mason from comment #43)
> @Jorg, the #ifdef MOZ_THUNDERBIRD is to avoid changing the behavior of the
> function in Firefox?
Yes.
> (Is mozilla/dom/base/nsDocumentEncoder.cpp shared between Firefox and
> Thunderbird?)
Yes.
(You could always read the comment at
https://dxr.mozilla.org/comm-central/source/mozilla/dom/base/nsDocumentEncoder.cpp#1443)

> In that case, how to solve the problem for Seamonkey (which has messenger +
> browser)?
> It would need to be a run-time decision?
You can't have it both ways, read the comment to understand the consequences for the browser part.

If someone is interested, submit a core patch that changes
  #ifdef MOZ_THUNDERBIRD
to
  #if defined(MOZ_THUNDERBIRD) || defined(MOZ_SUITE)

Comment 47

[Kent James (:rkent)]

Once we get a release of Thunderbird 45 beta that includes the patch from bug 1214377, but that would put Thunderbird 38.7.0 as the earliest release. That's probably too late to consider.