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)

VERIFIED FIXED in Firefox 68

Status

()

defect
P3
normal
VERIFIED FIXED
4 years ago
Last month

People

(Reporter: wross, Assigned: mbrodesser)

Tracking

({parity-chrome, regression})

Trunk
mozilla68
Points:
---
Dependency tree / graph
Bug Flags:
webcompat ?

Firefox Tracking Flags

(Webcompat Priority:?, firefox-esr60 wontfix, firefox67 wontfix, firefox68 fixed)

Details

(Whiteboard: [webcompat], )

Attachments

(4 attachments)

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.
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.
(In reply to wross from comment #1)
> it seems to be broken by 38.

Suspect: bug 1113238
Blocks: 1113238
Component: Untriaged → Serializers
Keywords: regression
Product: Firefox → Core
Flags: needinfo?(ehsan)
This may be the same as bug 1162963...
See Also: → 1162963
Flags: needinfo?(ehsan)
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.
Flags: needinfo?(ehsan)
OS: Unspecified → All
Hardware: Unspecified → All
Version: 39 Branch → Trunk
Duplicate of this bug: 1193153
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.
Duplicate of this bug: 1162963
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
Summary: Copying text with the style "white-space: pre;" does not preserve white space → Copying text with the style "white-space: pre;" does not preserve white space (spaces, tabs, newlines)
Duplicate of this bug: 1187476
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.
Blocks: 1193153
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?
Duplicate of this bug: 1195223
No longer blocks: 1193153
Duplicate of this bug: 1193153
Duplicate of this bug: 1199677
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.
See Also: 1162963
Summary: Copying text with the style "white-space: pre;" does not preserve white space (spaces, tabs, newlines) → Copying text with the style "white-space: pre;" does not preserve white space (spaces, tabs, newlines) (Pretty old bug 116083 has come back agai after change by bug 1113238)
Summary: Copying text with the style "white-space: pre;" does not preserve white space (spaces, tabs, newlines) (Pretty old bug 116083 has come back agai after change by bug 1113238) → Copying text with the style "white-space: pre;" does not preserve white space (spaces, tabs, newlines) (Pretty old bug 116083 has come backi after change by bug 1113238)
Summary: Copying text with the style "white-space: pre;" does not preserve white space (spaces, tabs, newlines) (Pretty old bug 116083 has come backi after change by bug 1113238) → 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 1113238)
See comment #18: Bug 1151873 brought back the unwanted behaviour.
Summary: 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 1113238) → 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)
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?
Yes.
Duplicate of this bug: 1249028
Flags: needinfo?(ehsan)
See Also: → 1357080
Simple workaround for this issue:

Start or end the selection outside of the element with the "white-space: pre" property. Then whitespace is preserved.
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...
(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
(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.
Priority: -- → P3
Duplicate of this bug: 1464664
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.
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.
Duplicate of this bug: 1390115
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

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.

Flags: webcompat?
Whiteboard: [webcompat]

(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.

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

Flags: needinfo?(masayuki)

Yeah, yes it is...

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

Flags: needinfo?(masayuki)
Keywords: parity-chrome

hsinyi:

Do you know somebody can work on this?

Flags: needinfo?(htsai)

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!

Flags: needinfo?(htsai)

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.

Assignee: nobody → mbrodesser

(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.

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.

@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:

Investigation to be continued.

When manually copying, SelectionCopyHelper calls nsClipboard::SetData.

The same two calls happen when the tests copies.

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.

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

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.

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.

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

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

(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.

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

Webcompat Priority: --- → ?

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

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).

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

(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.

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

Pushed by aiakab@mozilla.com:
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

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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.

Status: RESOLVED → VERIFIED
Blocks: 1551707

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.

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

Great job, indeed :-)

You need to log in before you can comment on or make changes to this bug.