1174452 - Copying text with the style "white-space: pre;" does not preserve white space (spaces, tabs, newlines) (Pretty old bug 116083 has come back after change by bug 1151873)

Status: VERIFIED FIXED

(Core :: DOM: Serializers, defect, P3)

Description

[Will Ross]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.30 Safari/537.36

Steps to reproduce:

Copy text with the "white-space: pre;" style that includes line breaks and/or multiple  contiguous sequences of white space.


Actual results:

The line breaks and white-space are preserved.


Expected results:

White space is collapsed.

Comment 1

[Will Ross]

This seems to be a regression of https://bugzilla.mozilla.org/show_bug.cgi?id=116083. I confirmed that bug was fixed in 37, but it seems to be broken by 38.

Comment 2

[Gingerbread Man]

(In reply to wross from comment #1)
> it seems to be broken by 38.

Suspect: bug 1113238

Comment 3

[(no longer active)]

This may be the same as bug 1162963...

Comment 4

[(no longer active)]

Attachment: what I have so far

This is a huge mess.

What is happening is that when copying text/html, we correctly preserve the whitespace, but there is nothing to tell the target when pasting that the text comes from a white-space: pre element, or such.  This is why, for example, everything works correctly when performing "select all", but not when you select the contents of the pre-formatted section without selecting the prefromatted node itself.

This is what I have so far, but it still doesn't work well enough.  This will add a style="white-space: pre;" to the context that we put into the text/_moz_htmlcontext" flavor, but that is useless when pasting.

I guess I need to massage this code a bit more to wrap the contents of what we copy in the ancestor element if it's a preformatted text node, so that we'd have somewhere to stick our white-space: pre style.

And I haven't even began to look at what we do when copying the text/plain flavor yet.

Comment 5

[Robin Whittle]

Here is what I wrote to https://bugzilla.mozilla.org/show_bug.cgi?id=1151873.  I was referred there to this bug.

I could not find a specific bug for the following behavior so am writing here in the hope that this is caused by this bug.

In Thunderbird 38.0.1 (Windows 7), using the plain text editor (I don't use the HTML editor) for email, if I have text with two or more spaces between words, then when I select this and copy it to the clipboard, the multiple spaces are replaced by a single space.

  xx   yyy    zzzz

becomes

 xx yyy zzzz

This occurs in the copy operation, since the results can be seen by pasting into a separate program.  There's no problem with pasting from another program, or with saving and re-opening the message as a draft.

Additional notes for this bug:

The same behavior occurs for a Control A select all selection or for using the cursor to select a subset.  I am not using 38.1.0 because it would not work with my IMAP server and I didn't feel like spending an unknown amount of time with different settings trying to make it work.

Comment 6

[Robin Whittle]

I just discovered (with Thunderbird 38.1.0) that when replying to a message, if I select multiple lines of text which begins with a '>' and copy this to the clipboard, when I paste this (into Thunderbird or anywhere else), the multiple spaces between words are preserved but all the line feeds disappear, so, for instance:

> 12    789
> 123456789

becomes:

> 12    789> 123456789

If someone suggests this behavior is not a result of this or any other bug, I will create a new one.

Comment 7

[Jorg K (CEST = GMT+2)]

(In reply to Robin Whittle from comment #6)
> I just discovered (with Thunderbird 38.1.0) that when replying to a message,
> if I select multiple lines of text which begins with a '>' and copy this to
> the clipboard, when I paste this (into Thunderbird or anywhere else), the
> multiple spaces between words are preserved but all the line feeds
> disappear

That's bug 1143570, whose fix was included in TB 38.1 via bug 1173261, see there.
Overloading this bug here with other issues will hinder it getting fixed.

Comment 8

[Jorg K (CEST = GMT+2)]

Just a brief history of the CSS "white-space: pre;" problem.

This was fixed in bug 116083 bug seems to have regressed in bug 1151873.
Also see the comment on the changeset: https://hg.mozilla.org/mozilla-central/rev/47d62ded4e5f

Comment 9

[Robin Whittle]

It was not clear to me that these were separate problems.  I have commented in bug 1173261.

I just installed Early Bird Thunderbird (Windows 7, installed from 40.0b1-candidates/build3/win32/en-GB/ an hour or two ago, which I am able to use thanks to what I learned about SSL in bug 1184488) and the behavior is still the same: when copying to the clipboard ordinary text in the plain text editor (not blue quoted and potentially subsequently edited text), the clipboard result retains newlines but converts all instances of two or more consecutive spaces to a single space, or, if there is a newline followed by one or more spaces, all those spaces are lost.

Comment 10

[Jorg K (CEST = GMT+2)]

Yes, this is why this bug here exists. See also bug 1162963 which is most likely the same problem.

Comment 11

[Robin Whittle]

OK - this bug's behavior also exists in http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-comm-central-l10n/ thunderbird-42.0a1.en-GB.win32.installer.exe 25-Jul-2015 12:41.

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: Simple page to show the bug

Select the
===
Multiple spaces:
aa      bb      cc
===
or
===
Tabs:
	aa
	bb
	cc
===
and copy it to a text editor (Notepad, etc.). Whitespace (incl. tabs) are lost. 

As Ehsan described in comment #4, select all and paste works.

Comment 15

[Jorg K (CEST = GMT+2)]

Confirming regression:
Last good: 2015-04-09
First bad: 2015-04-10
Both tabs and spaced get lost when copying as can be seen in attachment 8646518 [details].

Regressed by: https://hg.mozilla.org/mozilla-central/rev/47d62ded4e5f

Comment 17

[Jorg K (CEST = GMT+2)]

Note that the Thunderbird "plain text" mode is nothing but a HTML editor where everything has the following CSS style: font-family: -moz-fixed; white-space: pre-wrap; width: 72ch;
That's why the duplicates of this bug come from Thunderbird users.

Comment 18

[Jorg K (CEST = GMT+2)]

There has really been a funny history to this:

First there was bug 116083 that complained that "white-space:pre" didn't work on copy paste.
This patch fixed it: attachment 8530567 [details] [diff] [review]:
https://hg.mozilla.org/mozilla-central/rev/d5ef728a519d
Instead of testing only for "pre-wrap" (as used by Thunderbird), it also tested to other types of "pre".

Then came bug 1151873, which complained that formatting within "pre", like bold, was not copied.
As a result, the new code from bug 116083 was completely removed, even removing the "pre-wrap" case:
Attachment 8590048 [details] [diff].
https://hg.mozilla.org/mozilla-central/rev/47d62ded4e5f

That of course undid the fix for bug 116083 and worse, it also removed the "pre-wrap" handling that had always worked. The question is: Why did the test not trigger that came with the fix in bug 116083?

Comment 22

[WADA:World Anti-bad-Duping Agency]

FYI.
Difference between "pre" and "pre-wrap" of white-space in CSS.
  https://developer.mozilla.org/en-US/docs/Web/CSS/white-space
    pre      : Sequences of whitespace are preserved,
               lines are only broken at newline characters in the source and at <br> elements.
    pre-wrap : Sequences of whitespace are preserved.
               Lines are broken at newline characters, at <br>, and as necessary to fill line boxes.

Comment 23

[Jorg K (CEST = GMT+2)]

See comment #18: Bug 1151873 brought back the unwanted behaviour.

Comment 24

[WADA:World Anti-bad-Duping Agency]

History looks:
 1. Bug 116083 was kept open for long time, and many bugs were duped, and patch was landed on version=37.
 2. And Bug 1151873 was opened for version=37, and patch was landed on version=38.
 3. Then Bug 1174452 (this bug) was opened for version=38.
Is it right?

Comment 25

[Jorg K (CEST = GMT+2)]

Yes.

Comment 27

[torhu]

Simple workaround for this issue:

Start or end the selection outside of the element with the "white-space: pre" property. Then whitespace is preserved.

Comment 28

[FFuser]

I can't copy and paste comments from YouTube.
I can't copy and paste Tweets from Twitter.

Line breaks and spaces of YouTube comments or Tweets disappear.


Is there any way to solve both this bug and Bug 1151873...?

If there is not, then mind as well solve THIS bug rather than Bug 1151873, I wish.


That's because YouTube and Twitter have a great influence.
If Firefox cannot handle these contents correctly, the situation is fatal.
Should be given priority over rich text...

Comment 29

[Gingerbread Man]

(In reply to FFuser from comment #28)
> I can't copy and paste comments from YouTube.

Bug 1466391.

> I can't copy and paste Tweets from Twitter.

No existing report that I can find. If you can reproduce the issue in a brand new profile, please file one.
https://support.mozilla.com/kb/profile-manager-create-and-remove-firefox-profiles
https://developer.mozilla.org/docs/Mozilla/QA/Bug_writing_guidelines

Comment 30

[FFuser]

(In reply to Gingerbread Man from comment #29)
> (In reply to FFuser from comment #28)
> > I can't copy and paste comments from YouTube.
> 
> Bug 1466391.
> 
> > I can't copy and paste Tweets from Twitter.
> 
> No existing report that I can find. If you can reproduce the issue in a
> brand new profile, please file one.
> https://support.mozilla.com/kb/profile-manager-create-and-remove-firefox-
> profiles
> https://developer.mozilla.org/docs/Mozilla/QA/Bug_writing_guidelines


Thank you for replying.

Here's a repro video.　The version of Firefox is 60.0.2 (64 bit).
https://drive.google.com/open?id=10FNyZJL1teBWHw16ZOsie_B7Vq5LVhFO

Line breaks and spaces of YouTube comments or Tweets disappear certainly.

Comment 32

[piotrkakol]

Don't know if this is helpful but I can still reproduce this bug in ProtonMail.

To do it, send someone an email in plaintext mode (arrow on the right in the composer) and when you access that email, try to copy it and paste somewhere (even outside Firefox) the endlines will change to spaces.

As mentioned above, the hotfix is to select also part of the previous/next email message.

Comment 33

[nyanpasu64]

Is this equivalent to https://bugzilla.mozilla.org/show_bug.cgi?id=1390115 ?

On Discord (after a recent Discord change?), this bug causes messages with line breaks (shift+Enter) to be collapsed into single lines.

Comment 35

[Jorg K (CEST = GMT+2)]

Same issue.

I once upon a time implemented a Thunderbird-only hack. A full solution for Gecko is still pending. For some light reading visit:
https://searchfox.org/mozilla-central/rev/21588b2a9824e0758fe11d10065e2c01ea9f32be/dom/base/nsDocumentEncoder.cpp#1244

Comment 36

[Karl Dubost💡 :karlcow]

Simple test case

data:text/html,<div style="white-space: pre-line;">foo%0Abar</div>

1. Copy and Paste with Chrome
2. Copy and Paste with Firefox

Chrome preserves the LINE FEEDs
Firefox removes any formatting.

Comment 37

[FFuser]

(In reply to Gingerbread Man from comment #29)

Most websites use "white-space: pre-wrap" for comments section.
Twitter and YouTube are no exception.
These sites also use "white-space: pre-wrap".

The problem that I can't copy and paste YouTube comments or Tweets apparently come from this bug.
So I needn't file it as a new issue.

Evidence screenshots:
https://drive.google.com/open?id=1LhYCNre0X9VLOC4pi6Rlmfi9SA-AwS5z
https://drive.google.com/open?id=1BKoePAov_81CPmz8h-djmf9hCu3MFv6A

I sometimes copy a comment and paste it on online chat, for quotation.
Then I meet with this bug. Very annoying.

As I said, these sites have great influence.
So fixing this bug is in urgent need.

Comment 38

[Jorg K (CEST = GMT+2)]

Masayuki-san, given the amount of complaints here and given that this is a webcompat issue, do you think some resource could be allocated to this issue even though Core::Serialisers is still unowned after all these years?

The problem does not exist in Thunderbird since we have some conditional code here which also summarises the issue:
https://searchfox.org/mozilla-central/rev/ddd1679c0534f7ddf36cafddd17b710c4fefe3c4/dom/base/nsDocumentEncoder.cpp#1129

Comment 39

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yeah, yes it is...

And I really don't have much time to working on such new complicated module for now.

Comment 40

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

hsinyi:

Do you know somebody can work on this?

Comment 41

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Thanks for raising this issue again. Let me take an action to explore alternatives and see how this could fit in our team roadmap. Thanks again!

Comment 42

[Stijn Herreman]

If I understand correctly, this bug is the cause of a copy-paste issue on GitHub. When copy-pasting a part of a file's content, the last line loses its indentation.

An example using https://github.com/mozilla/gecko-dev/blob/201450283cddc9e409cec707acb65ba6cf6037b1/README.txt:

• When completely selecting lines 10 to 13, line 13 is not indented.
• When selecting up to the start of line 14, line 13 is indented.

This matches the behaviour described in comment 27.

Comment 43

[Mirko Brodesser (:mbrodesser-Igalia)]

(In reply to Jorg K (GMT+2) from comment #18)

There has really been a funny history to this:

First there was bug 116083 that complained that "white-space:pre" didn't
work on copy paste.
This patch fixed it: attachment 8530567 [details] [diff] [review]:
https://hg.mozilla.org/mozilla-central/rev/d5ef728a519d
Instead of testing only for "pre-wrap" (as used by Thunderbird), it also
tested to other types of "pre".

Then came bug 1151873, which complained that formatting within "pre", like
bold, was not copied.
As a result, the new code from bug 116083 was completely removed, even
removing the "pre-wrap" case:
Attachment 8590048 [details] [diff].
https://hg.mozilla.org/mozilla-central/rev/47d62ded4e5f

That of course undid the fix for bug 116083 and worse, it also removed the
"pre-wrap" handling that had always worked. The question is: Why did the
test not trigger that came with the fix in bug 116083?

Thanks for the analysis. Indeed, an interesting question, why that test (dom/base/test/test_bug116083.html) passed and still passes. Findings so far:

• The expected value of the test is correct, at least for the first pre element.
• SimpleTest.waitForClipboard passes, so it seems to be buggy.

Investigation to be continued.

Comment 44

[Jorg K (CEST = GMT+2)]

Mirko, I'm sure you read comment #35 pointing to the TB hack what was added in bug 1214377 here:
https://hg.mozilla.org/mozilla-central/rev/c9bc8f46ac3e

There's also bug 1141786 which talks about undoing hacks from bug 1125956. But that hack as morphed into what I mentioned in the first paragraph of this comment, so bug 1141786 can be closed when it's all fixed.

Comment 45

[Mirko Brodesser (:mbrodesser-Igalia)]

@Jorg: thanks for the reminder. I first want to fix the test, as it's a necessity for fixing the bug and it helps understanding the corresponding code area as well.

Analysis so far:

• The test calls SimpleTest.waitForClipboard which calls SimpleTest.promiseClipboardChange which calls SpecialPowers.getClipboardData which calls getClipboardData which calls nsClipboard::GetData.
• When manually copy-pasting text nsClipboard::GetData is not called.
• Interestingly, the test fails when removing all divs for the test except the first one. In that case, the copied value contains line breaks. This is probably a separate problem.

Investigation to be continued.

Comment 46

[Mirko Brodesser (:mbrodesser-Igalia)]

When manually copying, SelectionCopyHelper calls nsClipboard::SetData.

The same two calls happen when the tests copies.

Comment 47

[Mirko Brodesser (:mbrodesser-Igalia)]

More information in order to fix the test:

When manually selecting the text foo bar of the test:

• textHTMLBuf becomes u"foo bar".
• Selecting is done by first calling Selection::Collapse followed by Selection::Extend.
• Selection::Collapse(nsINode* aContainer, [...]) is called via: nsFrameSelection::HandleClick -> nsFrameSelection::TakeFocus; nsINode is a nsTextNode.

When the test selects:

• It selects the div's children
• Selection::Collapse and Selection::Extend aren't called, which is expected.
• textHTMLBuf becomes u"<div style=\"white-space: pre\">foo bar</div>"

That is, the Selection of the test and the manual behavior differ. The nsTextNode containing foo bar seems invisible to the test. It's currently unclear if the test could mimic the manual behavior. To be figured out.

Comment 48

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1174452: Part 1 -- test collapsing and extending selection before copy-pasting for `style="white-space: pre`

This closer mimics user behaviour, compared to the existing tests using
getSelection().selectAllChildren.

Comment 49

[Mirko Brodesser (:mbrodesser-Igalia)]

The test added in https://bugzilla.mozilla.org/attachment.cgi?id=9060736 mimics the user behavior of selecting and copy pasting text. It currently fails.

Comment 50

[Mirko Brodesser (:mbrodesser-Igalia)]

Analysis why copying text styled with white-space: pre doesn't preserve consecutive blanks when manually copy-pasting such text:

1. In nsPlainTextSerializer::Write IsInPre() returns false. The same method afterwards 'compresses' consecutive blanks.
2. IsInPre() returns false, because nsPlainTextSerializer::ScanElementForPreformat isn't called.
3. The latter isn't called from nsDocumentEncoder::SerializeNodeStart because when manually selecting the text, aNode is of type nsTextNode which isn't an Element (hence aNode->IsElement() returns false). mNeedsPreformatScanning is set to true.
4. When selecting the div element encompassing the text to be selected (e.g. by getSelection().selectAllChildren(div) as the old test did), nsDocumentEncoder::SerializeToStringRecursive is called before. It calls SerializeNodeStart, with the div element. As that's indeed an element, ScanElementForPreformat is called. Recursively, nsPlainTextSerializer::Write is called for the nsTextNode node child of the div element with IsInPre() == true. Hence, the blanks aren't "compressed" in that scenario.

In order to fix this (ignoring potential rich formatting problems -- if still existent, we'll deal with them separately) one option is to ensure ScanElementForPreformat is also called for nsTextNode. ScanElementForPreformat should then be renamed to sth like ScanNodeForPreformat. Investigation to be continued.

Comment 51

[Jorg K (CEST = GMT+2)]

Reads like a crime novel, keep it coming ;-)

Comment 52

[Mirko Brodesser (:mbrodesser-Igalia)]

Attachment: Bug 1174452: Part 2 -- scan `nsTextNode`'s parent element for preformat in nsDocumentEncoder

This takes into account white-space: pre style of nsTextNode's
parent element when formatting the selected text of the nsTextNode
node.

Depends on D28850

Comment 53

[Jorg K (CEST = GMT+2)]

What about
https://searchfox.org/mozilla-central/rev/ddd1679c0534f7ddf36cafddd17b710c4fefe3c4/dom/base/nsDocumentEncoder.cpp#1129
Can that go now?

Comment 54

[Mirko Brodesser (:mbrodesser-Igalia)]

(In reply to Jorg K (GMT+2) from comment #53)

What about
https://searchfox.org/mozilla-central/rev/ddd1679c0534f7ddf36cafddd17b710c4fefe3c4/dom/base/nsDocumentEncoder.cpp#1129
Can that go now?

Probably yes. The part 2 review is just a first version, it's not final yet. Some other tests have to be adapted too.

Comment 55

[Firefox Bug Husbandry Bot]

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Comment 56

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating whiteboard priority tags to program flags.

Comment 57

[Mirko Brodesser (:mbrodesser-Igalia)]

The test and fix are ready. To be on the safe side, they'll be landed after the soft code freeze (originally planned for 6th of May, delayed to 13th of May).

Comment 58

[Jorg K (CEST = GMT+2)]

Hmm, to the old bad state will ride another ESR train for a year? And what about this:
https://searchfox.org/mozilla-central/rev/ddd1679c0534f7ddf36cafddd17b710c4fefe3c4/dom/base/nsDocumentEncoder.cpp#1129

Comment 59

[Mirko Brodesser (:mbrodesser-Igalia)]

(In reply to Jorg K (GMT+2) from comment #58)

Hmm, to the old bad state will ride another ESR train for a year? And what about this:
https://searchfox.org/mozilla-central/rev/ddd1679c0534f7ddf36cafddd17b710c4fefe3c4/dom/base/nsDocumentEncoder.cpp#1129

We first need more confidence that the proposed change doesn't cause any noticeable performance regression. That's why we want to first have it in Nightly. Once we gained that confidence, we can remove the hack in a separate ticket and commit.

Comment 60

[Mirko Brodesser (:mbrodesser-Igalia)]

Requesting checkin. A corresponding try run has only unrelated failures.

Comment 61

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe847bf28976
Part 1 -- test collapsing and extending selection before copy-pasting for style="white-space: pre r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/66cff9aa39bc
Part 2 -- scan nsTextNode's parent element for preformat in nsDocumentEncoder r=masayuki,hsivonen

Comment 62

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/fe847bf28976
https://hg.mozilla.org/mozilla-central/rev/66cff9aa39bc

Comment 63

[Jorg K (CEST = GMT+2)]

OK, I can verify this fix :-)

I've visited https://jsfiddle.net/q0ybk99t/3/ was per bug 1390115 comment #0 and copied stuff from the <div style="white-space:pre-wrap;"> to Notepad++ on Windows. In FF 60 ESR (sorry, not on current release) the white-space is collapsed, with a build with the fix, it's maintained.

So VERIFIED.

Now the test in Thunderbird: Our so-called plain-text mail windows have <body style="font-family: -moz-fixed; white-space: pre-wrap; width: 72ch;">. So the issue for TB users was to copy plain-text mail content to a text editor.

I removed the block at
https://searchfox.org/mozilla-central/rev/ddd1679c0534f7ddf36cafddd17b710c4fefe3c4/dom/base/nsDocumentEncoder.cpp#1129
and copied from a TB plain-text windows to Notepad++. It still works, hooray. I'll file a bug to remove that code then.

Comment 64

[Lars Viklund]

My problem case for this bug has been copying pre-formatted text from the RT bug tracker.

I have no idea where my original report is in the dupe swamp, but a public RT page that used to have the problem can be found at: https://rt.perl.org/Public/Bug/Display.html?id=134080#txn-1630821

I'm happy to report that it does no longer occur with a tip build that has the fix, both on that public tracker and our own internal one on which I originally encountered the problem.

Comment 65

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Great job, Mirko!
And thanks all for the verification!

Comment 66

[Jorg K (CEST = GMT+2)]

Great job, indeed :-)