Page MenuHomePhabricator

Commit emails have broken formatting
Closed, InvalidPublic

Assigned To
None
Authored By
aristedes
Jun 15 2016, 11:26 AM
Referenced Files
F1687893: Screen Shot 2016-06-15 at 5.54.19 AM.png
Jun 15 2016, 12:54 PM
F1687878: Screen Shot 2016-06-15 at 5.49.42 AM.png
Jun 15 2016, 12:54 PM
F1687765: Screen Shot 2016-06-15 at 8.58.39pm.png
Jun 15 2016, 11:26 AM
Tokens
"Like" token, awarded by anarut."Like" token, awarded by artyom.kravchenko.91."Like" token, awarded by AlexandrShestak."Like" token, awarded by george.filipovich."Like" token, awarded by akoyro.

Description

  1. svn external repository
  2. herald rule to send commit emails to group of users
  3. set up commit email to send diff and commit message in an html format

Problem:

  • Email contains the commit message twice. Once partially, and then again in full.
  • Formatting is erratic
  • Second sentence of the commit message is in a fixed width font which doesn't wrap (at least in some email clients) and so almost impossible to read.

This task is related to T11029 (improving diff email), but I think it is a separate and more urgent bug to fix rather than a long term improvement.

I'd be happy to propose a diff/pull request for improved layout if that's helpful, but I might not know about all the use-cases for this email and the way people want to use it.

I attach a screenshot of a commit email as shown on Thunderbird on OSX. You can clearly see the three different fonts/sizes, truncated and duplicated commit message and oddly formatted second paragraph.

Screen Shot 2016-06-15 at 8.58.39pm.png (524×1 px, 105 KB)

Event Timeline

Can you provide:

  • the entire raw commit message from SVN?
  • the raw body of the email you received (redact any sensitive information, I'm only interested in the markup)?
  • the raw body of the email Phabricator sent (find this mail in bin/mail list-outbound, then use bin/mail show-outbound --id <id> to extract the raw body; redact any sensitive information, I'm only interested in the markup).

Here's what this mail is generally expected to look like:

Screen Shot 2016-06-15 at 5.49.42 AM.png (403×877 px, 72 KB)

The middle section is the commit message rendered as remarkup, so it's expected that if you put **asdf** in a commit message you'll receive mail with bold text. It's possible that "We use cayenne..." might be indented two spaces in the raw message, then be formatted as a code block (giving it a monospaced style), and then not rendered with the background/border for some reason (because of your client?).

In most clients (here, Mail.app) code blocks should render in a style similar to the style used in the web UI, with monospaced text but on a grey background:

Screen Shot 2016-06-15 at 5.54.19 AM.png (176×228 px, 17 KB)

I've learned some interesting things.

The email I received did not have mime parts. Instead it had these headings

MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Type: text/html; charset="utf-8"

and was completely base64 encoded. I'll look for a tool to decode it if you need, however I was able to extract the email from the server logs.

I note that my email settings are for HTML, but phabricator sent no plain text part as well.

From the server-side tool (neat tool by the way!), I extracted this:

<table><tr><td style="">andrey committed rC43345: #28898 Course.getRelatedToCourses() functionality was moved to CourseService..
</td></tr></table><br /><div><div><p>#28898 Course.getRelatedToCourses() functionality was moved to CourseService.getRelatedCoursesFor(). Now this functionality does not use cayenne relations feature to get the related courses.</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px &quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">We use cayenne query to get these courses directly for database. It allows exclude any influence of the cayenne snapshot implmentation on these relations</pre></div></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>/willow/trunk/common/common-model/src/main/java/ish/oncourse/model/Course.java<br />
/willow/trunk/common/common-model/src/test/java/ish/oncourse/model/CayenneRelationTest.java<br />

I did discover that indeed there are two spaces at the start of the second paragraph. So remarkup is the culprit. It would be nice to turn that off since it is a little surprising in a commit message. Although it isn't too hard to avoid now we know it is there.

As for the additional (truncated) first line. Perhaps that makes more sense in a git world where it is common practice to have a short first line, limited to maybe 100 characters. I know github strongly encourage this sort of behaviour.

It would be nice to disable this behaviour for those who don't want the commit message repeated or truncated. Even in your example above, the commit message is repeated.

Finally, the change in fonts looks like it is due to the first time we see the commit message is it wrapped in a one row one cell table. And the second time it is wrapped in two divs and a p.

For my part I'd love to just get rid of that that first table and everything inside it. The author and commit number are in the email subject and from.

Thanks
Ari

I know I didn't retrieve all the info you asked for, so let me know if you want to see anything else or want me to decode the base64 email body.

<pre class="remarkup-code" style="font: 11px/15px &quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">We use cayenne query to get these courses directly for database. It allows exclude any influence of the cayenne snapshot implmentation on these relations</pre>

It appears that either your client just isn't rendering the background here, or possibly your MTA is mangling the mail in transit. But everything on our side appears to be working as intended.

Here are some steps you could take to change the behavior:

  • Use a different client which supports a broader range of HTML in mail bodies, or configure your client to support backgrounds if this is a feature you've disabled.
  • Switch to plain text mail in Settings.
  • Switch the entire install to plain text mail in Settings.
  • Disable any mail mangling options in your MTA pipeline.
  • If you dislike the summary behavior, you can fork Phabricator to change it. The behavior is desirable for the overwhelming majority of users and repositories and I have no plans to add options for it (see also T8227).
  • If you dislike the mail layout behavior, you can fork Phabricator to change it. I have no plans to add options for it.

I know you have focussed on the missing background, but I'm not overly fussed about that. But I am surprised that the commit subject shown twice in a row, once truncated inside a table and once not, is anything else but a bug.

Users really want to read the same thing twice like that?

Well, its your project. As soon as I find the correct template file I'll adjust it and be happy.