Page MenuHomePhabricator
Create Task
Maniphest T9896

make images from comments visible in email
Open, Needs TriagePublic
Actions

Description

When pasting an image into a comment, the email renders something like F424: Screen Shot 2015-11-06 or {F424}.
Users with to see the content of the photo in their email.

We probably can't reasonably just attach the photo (That could end up big), but embedding <img> tag for a direct-download of the image might work.

For reference, GH uses the "Everyone with the link" security model (i.e., there's an no-security obfuscated link to the content). This might be a little extreme, but it gives users the behavior they like.

(Actually, this might already be possible using some combination of configuration I'm not thinking about).

Related Objects

Event Timeline

avivey created this task.Dec 3 2015, 9:18 PM
avivey raised the priority of this task from to Needs Triage.
avivey updated the task description. (Show Details)
avivey added projects: Mail, Remarkup.
avivey added a subscriber: avivey.
joshuaspence added a subscriber: joshuaspence.Dec 4 2015, 11:57 AM
avivey added a comment.Dec 17 2015, 11:42 PM

This is closely related to T6670.

avivey added a project: Restricted Project.Dec 23 2015, 1:15 AM
avivey moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Dec 23 2015, 7:21 PM
avivey added a comment.Jan 14 2016, 6:29 PM

We probably can't reasonably just attach the photo (That could end up big)

We might want to do this regardless of costs, for VPN issues...

epriestley closed this task as a duplicate of T10262: Scramble file secrets when attached objects change their view policies.Feb 2 2016, 1:08 AM
epriestley mentioned this in T10262: Scramble file secrets when attached objects change their view policies.Feb 2 2016, 1:18 AM
avivey added a comment.Sep 22 2016, 10:06 PM

Did this ever actually work? it looks like we're still embedding links:

pasted_file (127×363 px, 7 KB)

Herald added a subscriber: eadler. · View Herald TranscriptSep 22 2016, 10:06 PM
epriestley reopened this task as Open.Sep 22 2016, 10:12 PM
epriestley added a subscriber: epriestley.

I think this didn't actually get handled in T10262, but should be doable now.

hskiba added a subscriber: hskiba.Jul 28 2017, 5:12 PM
epriestley moved this task from Backlog to Soon? on the Mail board.Jan 2 2019, 3:21 PM
epriestley added a comment.Jan 15 2019, 2:45 PM

PhabricatorObjectRemarkupRule->renderObjectEmbedForAnyMedia() currently calls renderObjectTagForMail() unconditionally when we're in HTML mail mode. This always renders a (F123: Xyz) element.

Shortest path here is to replace that with some new renderObjectEmbedForMail() which falls back to renderObjectTagForMail(), then override the new renderObjectEmbedForMail() in PhabricatorEmbedFileRemarkupRule and render an <img />. After T10262, there's no longer an issue around generating a (relatively) stable URI which can be referenced externally.

However, all the call paths here are kind of a mess and implementing this cleanly (that is, not just copy/pasting a big chunk of PhabricatorEmbedFileRemarkupRule) doesn't seem straightforward, so I think this one is going back on the pile for next time.

epriestley moved this task from Soon? to Future on the Mail board.Jan 15 2019, 2:45 PM
Phabricator · Built by Phacility