After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 731746 - Empty lines removed in plaintext messages
Empty lines removed in plaintext messages
Status: RESOLVED FIXED
Product: geary
Classification: Other
Component: composer
0.6.x
Other Linux
: Normal normal
: ---
Assigned To: Robert Schroll
Geary Maintainers
review
Depends on:
Blocks:
 
 
Reported: 2014-06-16 20:36 UTC by Ramesh Dharan
Modified: 2014-06-19 20:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Message as received (29.88 KB, image/png)
2014-06-16 20:36 UTC, Ramesh Dharan
  Details
Message as sent (36.65 KB, image/png)
2014-06-16 20:37 UTC, Ramesh Dharan
  Details
Remove trailing whitespace before wrapping plain text (1.00 KB, patch)
2014-06-19 19:50 UTC, Robert Schroll
committed Details | Review

Description Ramesh Dharan 2014-06-16 20:36:21 UTC
Created attachment 278558 [details]
Message as received

I'm using Geary from the bleeding edge PPA (ppa:yorba/daily-builds). According to dpkg my version is 201406110122~0.6.0+1427~ubuntu14.04.1. 

When composing a plaintext message, everything looks fine in the composer window, but once sent the message seems to be missing empty lines between paragraphs. 

That is to say, I need to insert two lines between each paragraph in order for there to be one line as seen by the recipient, or as seen in the Sent Mail folder. 

See the attached screenshots which demonstrate the issue.
Comment 1 Ramesh Dharan 2014-06-16 20:37:01 UTC
Created attachment 278559 [details]
Message as sent
Comment 2 Robert Schroll 2014-06-16 21:13:55 UTC
How reproducible is this?  Is there a particular message that reliably triggers it?  (I tried sending a similar message to myself and didn't see a problem.)

We've had similar issues in the past (see bug #713990, bug #723815).  I had hoped that we'd killed all of them, but I'm not surprised to see it again.  The basic problem is that the composer is actually creating HTML and then turning that into plain text.  And for some reason, blank lines appear or disappear in this process.
Comment 3 Ramesh Dharan 2014-06-16 22:11:20 UTC
It happens 100% for me with plaintext messages in the current build I'm running. 

That makes sense re: the HTML->plain text conversion; kinda figured something like that was happening.

Note that in the Gmail Web UI as well the line breaks are missing, but I just noticed that in Evolution the message line breaks show up. 

Perhaps it's related to the format=flowed we set for the Content-Type: header? I noticed that other plain-text messages I send don't have that...
Comment 4 Ramesh Dharan 2014-06-19 19:03:17 UTC
I dug into this a little more and it looks like the line break is only removed if there's a trailing space on the previous line.

Not sure if that makes it any easier to "fix" but at least I understand the behavior now and can train myself to not leave any trailing space.
Comment 5 Robert Schroll 2014-06-19 19:23:22 UTC
(In reply to comment #4)
> I dug into this a little more and it looks like the line break is only removed
> if there's a trailing space on the previous line.

Aha!  A trailing space is a signal in the flowed-text plain text format that the paragraph continues on the next line.  I don't see any thing in RFC 3676 about encoding a line-ending space, so I think we should just delete this before sending the message through our flowed-text formatter.
Comment 6 Robert Schroll 2014-06-19 19:50:47 UTC
Created attachment 278801 [details] [review]
Remove trailing whitespace before wrapping plain text

This is overkill in some sense, since it'll remove whitespace other than 
spaces as well.  But that's a reasonable thing to do, I think, and it 
may help avoid other confusing situations.
Comment 7 Jim Nelson 2014-06-19 19:52:51 UTC
Review of attachment 278801 [details] [review]:

Commit!
Comment 8 Robert Schroll 2014-06-19 19:55:05 UTC
Attachment 278801 [details] pushed as 3895730 - Remove trailing whitespace before wrapping plain text
Comment 9 Ramesh Dharan 2014-06-19 20:38:22 UTC
Thanks for the quick turnaround! I applied the patch and verified it worked for me as well.