Page MenuHomePhabricator

Emails from Diffusion contain space or new line character in link making them not clickable
Closed, ResolvedPublic

Description

When Diffusion sends out an email about repository updates (for example, when a revision is closed), it contains several links to the server instance for more information.

Often for us (90% of the time estimated), there's a minor issue that occurs around the point of the link to the Phabricator server. This link contains a new line or whitespace character so the link is not clickable. Occasionally this space character is in the title instead of the link tag. This may be because there is a limit to the number of characters the mail client (Apple mail in this case) will use before it inserts a new line character.

Example email (scrubbed). The below example is an email that was copied and pasted and sensitive information was removed, however the issue is exact as shown. The recipient of this email would not be able to click on the link https://myserver.something.com/D12345 due to the end line.

This behavior does not happen all the time, only when there is a suffient amount of characters without a newline before the link. As mentioned above, sometimes the new line occurs after the link within the title and the mail client renders this as a space. In the example used previously, the link would look like https://myserver.so mething.com/D12345. It will still be clickable in the mail client though unlike the previous example.

A potential solution to the issue (just a guess) may be to just add an end line character at an appropriate spot before this link so it renders correctly.

Date: Mon, 26 Sep 2016 15:57:07 -0700
To: <scrubbed>
From: <scrubbed>
Subject: Re: [Differential] [Closed] D12345: <scrubbed>
Message-id: <scrubbed>
X-Priority: 3
X-Phabricator-Sent-This-Message: Yes
X-Mail-Transport-Agent: MetaMTA
X-Auto-Response-Suppress: All
X-Phabricator-Mail-Tags: <differential-updated>, <differential-committed>
X-Herald-Rules: <60>
X-Phabricator-Projects: <scrubbed>
X-Phabricator-To: <scrubbed>
Precedence: bulk
In-reply-to:
 <scrubbed>
References:
 <scrubbed>
Thread-index: <scrubbed>
--Boundary_(ID_oj3/m6Rep4skVmJAwianMA)
Content-type: text/html; CHARSET=US-ASCII
Content-transfer-encoding: 7BIT

<table><tr><td style="">This revision was automatically updated to reflect the committed changes.<br />Closed by commit <scrubbed></td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="<scrubbed>" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>CHANGES SINCE LAST UPDATE</strong><div><a href="<scrubbed>" rel="noreferrer">https://<scrubbed></a></div></div><br /><div><strong>REPOSITORY</strong><div><div>r<scrubbed></div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://myserver.someth
 ing.com/D12345" rel="noreferrer">https://myserver.something.com/D12345</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div><scrubbed><br />

[...]

--Boundary_(ID_oj3/m6Rep4skVmJAwianMA)--

Version:

phabricator 34af66122e47f3194156847e36e583b5d67cfcda (Aug 15 2016)
arcanist de1231c62a4fa4935a33e73dc95bb352ce40a0a0 (May 31 2016)
phutil 5a9becb0920ec3983925f3617f76933702a925d5 (Jun 7 2016)

Event Timeline

What does this show, when run in your local working copy?

phabricator/ $ git merge-base HEAD origin/master

We're running a fork and sorry, I know it's a little behind stable. We're working on getting back to a vanilla install to make updates easier.

$ git merge-base HEAD origin/master
33bf2a79def2e03c2248730afa080ead489942be

Hmm, I don't see that one in the upstream either. Maybe your origin is also a fork?

My best guess is that whatever you're forked from is behind c694bd651b49307dbc014deabbe40d6891e7e01a (which hit the upstream on Jun 9), and that upgrading past that will fix things. Alternatively, you can set phpmailer.smtp-encoding to base64 in Config, which has the same effect as upgrading. After making the config change, you may need to restart the daemons to make sure they pick it up.

I'll see if I can update the version information documentation (to discuss what to do if you're forked) and "Version" panel (to try harder to find a hash in the upstream).

We can best help you if you follow the suggestions in Contributing Bug Reports when reporting bugs. In particular:

  • If possible, upgrade to HEAD of master or stable first: many issues are fixed by the time they are reported, especially if you're 6+ months behind HEAD. Even if the issue isn't fixed yet and we patch it, you'll probably have to upgrade anyway.
  • Reproduce the issue against upstream versions, so we can rule out that local patches might be causing the issue. Although it's not likely that local changes cause problems, we're powerless to rule them out from the upstream.
  • When reporting versions, give us versions in the upstream. One way we use this information is to check if you're behind known fixes for similar issues, but we can't do this if the hash you give us isn't part of our history. I'll update the documentation to be more clear about this.

Let me know if that doesn't resolve things. I can't immediately reproduce this so it's definitely possible that I'm on the wrong track here, but I think upgrading is pretty likely to fix this since it sounds very similar to T11120 to me.

Thanks I made the smtp-encoding change. We're just about ready to update to a vanilla install. If it doesn't fix it and I can provide reproducible steps, I'll update this bug report.

epriestley claimed this task.

After D17103, at HEAD of master:

  • We'll now try to identify branchpoints from the upstream on the "Version Information" screen.
  • The documentation has been updated to discuss that we can only do anything useful with upstream hashes. The documentation may take 24 hours to update, so this change may not be reflected immediately.

I'm going to presume this is resolved, but feel free to follow up or file a new report if the configuration change and updating do not resolve things.