Ref T6343, adding HTMLMailMode to remarkup, and most objects should now be processed and appear pretty in emails.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T6343: Phabricator HTML emails have unclickable URLs in Thunderbird
Restricted Maniphest Task - Commits
- Restricted Diffusion Commit
rP1b438a8bd106: Process Remarkup in text and HTML email bodies appropriately
Add a comment to a Maniphest task containing a mention of an object like '{T1}' or 'T1'. Emails should show a styled version of the object similar to how the object looks in the context of the Maniphest task in the UI.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
We should fix the $viewer thing, see inline. Looks great otherwise.
| src/applications/metamta/view/PhabricatorMetaMTAMailBody.php | ||
|---|---|---|
| 47 | We should use the actual viewer for these (the user who is currently logged in). To make it available:
| |
| 53 | Same as above. | |
| src/applications/paste/mail/PasteCreateMailReceiver.php | ||
| 75 ↗ | (On Diff #26075) | This isn't technically remarkup, it's always just the string "You successfully created a paste." It doesn't really matter if we add this as a raw or remarkup section, but sticking with raw is maybe cleaner. |
| src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php | ||
| 2161–2163 | This is where you can setViewer($this->requireActor()) to make the appropriate user account available inside the mail body. | |
| src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php | ||
| 65–67 | Consider passing $status or $is_closed into renderObjectTagForMail() to clean this up a little bit more. | |
| src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php | ||
|---|---|---|
| 65–67 | I was debating this. My thought was that the standalone acetic html tag can be re-used for things besides emails, and perhaps closed might not be a property in that case. If you don't anticipate such a case, your way makes more sense to me. | |
Comment Actions
Yeah I don't know if the objects like T2000 need to look the same in email. I'd try just plain old anchor for now and revisit if needed.
Comment Actions
Okay, here are the issues I caught with rules that are actually broken; we should fix these:
- The icon rule ({icon paw} produces ) throws an exception. (This one is probably really hard to make work in HTML email.) I think it would be OK to just render {icon paw} or similar for now.
- The macro rule produces image tags with relative URIs (starting with / instead of http://...).
- The diviner rule (@{article:example}) produces links with relative URIs.
Here are the minor issues I caught with rules that are iffy, but not obvious to the user and don't need to be fixed for now:
- The nav sequence rule ({nav a > b > c}) works OK but produces markup with class attributes.
- The username mention rule (@username) produces markup with class attributes.
- The interpreter block rules like cowsay produces markup with class attributes.
See inline for the try/catch thing.
Comment Actions
durr
| src/applications/metamta/view/PhabricatorMetaMTAMailBody.php | ||
|---|---|---|
| 56 | In case we missed any exceptions, let's do this:
} catch (Exception $ex) {
phlog($ex);
$this->htmlSections[] = phutil_escape_html_newlines(phutil_tag('div', array(), $text));
}That will use the old mode (just dump the remarup into the mail as text) for anything we break on, and log the exception. This should prevent this from causing any major issues, and we can clean up the minor stuff ("this mail could look nicer") over time / as issues arise. | |
Comment Actions
Adding try/catch for html mail styling. Fixing a few rules to leave out class attributes when composing email body.